Monday, 2022-01-31

ykarelslaweq, hi08:30
ykarelwhen you get chance please check https://review.opendev.org/c/openstack/neutron-lib/+/82666808:30
ykareland https://review.opendev.org/c/openstack/neutron/+/826811 too08:31
ykarelThanks in advance08:31
slaweqykarel: sure08:46
slaweqI will finish one thing and will check those patches08:46
opendevreviewLuis Tomas Bolivar proposed openstack/neutron stable/xena: Use Port_Binding up column to set Neutron port status  https://review.opendev.org/c/openstack/neutron/+/82326708:48
opendevreviewSlawek Kaplonski proposed openstack/neutron master: DNM Add some extra logs to debug DHCP issue in TripleO CI  https://review.opendev.org/c/openstack/neutron/+/82699908:50
slaweqykarel: both patches reviewed09:00
ykarelThanks slaweq 09:01
ykarelslaweq, can u check https://review.opendev.org/c/openstack/neutron/+/826595 one too09:02
ykarelneeded in stable branches only09:02
ykarelwill backport to others once it's merged09:02
slaweqykarel: done09:03
ykarelThanks a lot09:03
slaweqyou need also lajoskatona, ralonsoh or bcafarel to check it09:03
ykarelyeap i think i added them09:04
ralonsohI'll take a look09:05
ykarelThanks ralonsoh 09:05
ykarelralonsoh, also have you got chance to look into ovs tempest issue?09:05
ykareli have seen multiple failures related to that09:06
ralonsohwhat issue?09:06
ykarellet me fetch link, the one raised in last CI meeting09:06
ykarelhttps://etherpad.opendev.org/p/neutron-ci-meetings#L13809:06
ykareli have seen those only stable/wallaby patches09:07
ykarelnot sure if other branches affected or not09:07
ralonsohno, I didn't check this09:07
ykarelok can you check this when get a chance09:08
ykareli think i will report lp for this as seen multiple occurance already09:08
ralonsohbut why another LP bug?09:15
ralonsohif there is one reported already09:15
ykareli don't think it's already reported09:15
ykarelhave u seen similar lp reported?09:16
ykarelralonsoh, reported https://bugs.launchpad.net/neutron/+bug/195956409:27
ykarelif some other is already known, can mark it duplicate09:27
* ykarel leaves for lunch09:28
lajoskatonaykarel: thanks09:39
*** dmellado_ is now known as dmellado09:45
opendevreviewLuis Tomas Bolivar proposed openstack/ovn-octavia-provider stable/xena: Support creating members without a subnet ID  https://review.opendev.org/c/openstack/ovn-octavia-provider/+/82701009:46
opendevreviewLuis Tomas Bolivar proposed openstack/ovn-octavia-provider stable/wallaby: Support creating members without a subnet ID  https://review.opendev.org/c/openstack/ovn-octavia-provider/+/82701109:51
opendevreviewMerged openstack/neutron-lib stable/stein: Dropping lower constraints testing (stable Xena)  https://review.opendev.org/c/openstack/neutron-lib/+/82666810:14
opendevreviewBence Romsics proposed openstack/neutron stable/xena: Disable tracebacks of eventlet.wsgi.server  https://review.opendev.org/c/openstack/neutron/+/82703710:19
opendevreviewBence Romsics proposed openstack/neutron stable/wallaby: Disable tracebacks of eventlet.wsgi.server  https://review.opendev.org/c/openstack/neutron/+/82703810:20
opendevreviewBence Romsics proposed openstack/neutron stable/victoria: Disable tracebacks of eventlet.wsgi.server  https://review.opendev.org/c/openstack/neutron/+/82703910:20
opendevreviewLuis Tomas Bolivar proposed openstack/ovn-octavia-provider stable/victoria: Support creating members without a subnet ID  https://review.opendev.org/c/openstack/ovn-octavia-provider/+/82704010:21
opendevreviewLajos Katona proposed openstack/tap-as-a-service master: DNM: test master  https://review.opendev.org/c/openstack/tap-as-a-service/+/82704310:28
opendevreviewLuis Tomas Bolivar proposed openstack/ovn-octavia-provider stable/ussuri: Support creating members without a subnet ID  https://review.opendev.org/c/openstack/ovn-octavia-provider/+/82704510:42
opendevreviewMerged openstack/neutron-lib stable/stein: Enforce policy for qos_policy_id attribute  https://review.opendev.org/c/openstack/neutron-lib/+/82661510:47
ykarelralonsoh, if you can also check if https://review.opendev.org/q/Ib6b70114efb140cf1393b57ebc350fea4b0a2443 can cause those issues in wallaby ovs jobs10:50
ralonsohykarel, there is a bug open related to this10:50
ralonsohhttps://review.opendev.org/c/openstack/neutron/+/82629110:50
ralonsohthis is the patch10:51
* ykarel looks10:52
ykarelralonsoh, ohkk that looks WIP, so simiilar needed in stable/wallaby too, ^ is proposed against stable/victoria?10:55
ykarelor we can revert stable/wallaby one until fixed10:56
ralonsohnot yet because the patch is not finished10:56
ralonsohthis is a security fix10:56
ralonsohI wouldn't revert a security patch10:56
ykarelokk i meant temporary revert, but ok10:57
ykarelas it's affecting gates10:57
opendevreviewLuis Tomas Bolivar proposed openstack/networking-ovn stable/train: Support creating members without a subnet ID  https://review.opendev.org/c/openstack/networking-ovn/+/82705111:02
rubasovykarel, ralonsoh: if you see various random timeouts in tests that check connectivity over dhcp and/or router ports, that patch can definitely be the reason11:12
rubasovfor some reason we did not see a single failure until the patch got backported to stable/victoria, but there it broke several dozen tests11:13
rubasovand after a recheck it breaks a quite different set of tests where the only common part is that those tests check for connectivity11:14
ralonsohrubasov, I'm trying to create a list of flow calls11:14
rubasovbut it should only break ovs based deployments11:14
ralonsohto address your last reply there11:15
ralonsohrelated to the flow deletion11:15
rubasovralonsoh: that's the sure way to know11:15
ralonsohwhy did you say the delete flow is executed in other transaction?11:15
rubasovif you see a flow with priority=65534 left around then that patch is guilty11:15
ralonsohhttps://review.opendev.org/c/openstack/neutron/+/826460/3/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py11:15
rubasovbecause add_flow runs in dhcp/l3-agent11:16
rubasovbut delete_flows runs in ovs-agent11:16
ralonsohrubasov, sorry, who is setting the dead vlan tag?11:17
rubasovdhcp/l3-agent11:17
rubasovfrom replace_port()11:17
ralonsohrubasov, can you point me to the code?11:18
rubasova moment11:19
rubasovhttps://opendev.org/openstack/neutron/src/commit/30951fcdfab753153a30c4e77309da0b9afd919e/neutron/agent/common/ovs_lib.py#L379-L38011:20
rubasov^ setting the dead vlan tag11:20
ralonsohrubasov, yes, but where in the DHCP code11:21
opendevreviewyatin proposed openstack/neutron stable/xena: [Xena Only] Switch to xena neutron-tempest-plugin jobs  https://review.opendev.org/c/openstack/neutron/+/82705711:21
rubasovhttps://opendev.org/openstack/neutron/src/commit/30951fcdfab753153a30c4e77309da0b9afd919e/neutron/agent/linux/dhcp.py#L1696-L170311:22
ralonsohrubasov, ok, so we are, sometimes, binding the device before we plug it?11:23
opendevreviewyatin proposed openstack/neutron stable/xena: [Stable Only] Enforce policy for qos_policy_id attribute  https://review.opendev.org/c/openstack/neutron/+/82659511:24
rubasovthat plug() call gets down to replace_port() which always sets the tag to the dead vlan tag11:25
rubasovI would not call that binding11:25
ralonsohrubasov, the point is that the DHCP agent creates the port11:26
opendevreviewLucas Alvares Gomes proposed openstack/neutron master: [OVN] Migrate "reside-on-redirect-chassis" for distributed FIP  https://review.opendev.org/c/openstack/neutron/+/82691211:26
ralonsohthe OVS agent binds it and removes the dead vlan tag11:26
rubasovralonsoh: yes, it does11:26
ralonsohbut sometimes this is not the order11:27
ralonsohand we call the dead vlan tag removal before writing it11:27
ralonsohpffff11:27
rubasovpretty much, but there's an important detail11:28
rubasovreplace port operates in an ovsdb transaction11:28
rubasovbut the attempted security fix also adds an extra flow right after that - which of course cannot be part of the ovsdb transaction since it's on openflow operation11:29
rubasovbut before creating the port we don't know the ofport so we cannot pre-create the flow11:30
rubasovso I think there's no race in the ovsdb part, only in the openflow part11:30
ralonsohyeah, we create the port with the tag and then we add the flow. At the same time that could be detected by the OVS agent that will try to bind it (and remove the flow)11:33
rubasovexactly11:34
rubasovand we need *something* to make the freshly plugged port actually dead (the access port tag + the dead vlan drop flow is not enough), otherwise the dead vlan leaks11:36
rubasovthe attempted security fix tried to use the push dead vlan tag flow for this something11:37
rubasovbut that lead to the race condition we have now11:38
rubasovregarding the comments you and Oleg left on https://review.opendev.org/c/openstack/neutron/+/82646011:43
rubasovI don't think strict=False will solve the problem, because that could change the timing on one process, but it still races with the other process11:43
ralonsohyes11:44
ralonsohyou are right on this11:44
rubasovand based on docs I also don't think any other vlan_mode would solve the problem11:44
ralonsohthe problem is between two processes11:44
rubasovbut I couldn't actually test that yet - I was just putting together a play-environment for that11:45
ralonsohno, that applies to flow commands in the same transaction11:45
ralonsohnow I realize this is because of a race condition between two agents11:45
rubasovbut my thinking is that the ovs openflow pipeline does not see the access port tag, because that's an important optimization in ovs - to modify the frame as late as possible11:46
rubasovand that optimization will apply to the other vlan modes too11:46
rubasovplus ovs-vswitchd.conf.db.5 says that all vlan_modes apply only to "normal switching"11:48
rubasovit would be great to make the freshly plugged port dead by an ovsdb attribute (and then it would be dead right away in the transaction that created it)11:54
rubasovbut is there any ovsdb attribute of a port that we can use for that purpose?11:54
ralonsohrubasov, I was looking for that12:03
rubasovI'm testing Oleg's ideas with the vlan_modes12:04
rubasovI think I checked all relevant vlan_modes and they behave the same as an access port, that is openflow rules don't see the vlan set in the port's tag attribute12:19
rubasov(replied to Oleg's comment in the review with the details)12:19
opendevreviewMarios Andreou proposed openstack/neutron master: DNM Revert "Call enable DHCP only if there are subnets with enabled DHCP in network"  https://review.opendev.org/c/openstack/neutron/+/82701412:30
ralonsohrubasov, OK, I think we can add the flow BEFORE the port creation12:36
ralonsohbut that implies a change in ovsdbapp12:36
ralonsohwe can provide the port UUID12:36
ralonsohone sec12:36
rubasovsounds interesting12:36
ralonsohhttps://github.com/openvswitch/ovs/blob/master/python/ovs/db/idl.py#L2000-L201612:37
ralonsohthis is the call made to the ovs library from ovsdbapp12:37
ralonsohwe can provide a uudi12:37
ralonsohnew_uuid = uuid.uuid4()12:37
ralonsohof course, that will imply a new ovsdbapp release12:38
ralonsohbut I think this is doable, I'll push a patch today12:38
rubasovlet's give it a try, that would eliminate the performance effect of the currently proposed patch12:42
ralonsohrubasov, my bad...12:42
ralonsohwhat we need is the ofport...12:42
ralonsohnot the port/interface UUID...12:43
rubasov:-(12:43
rubasovat some point I was also thinking of "ovs-ofctl mod-port down", but since it's ofctl that's likely also implemented as an openflow operation12:51
ralonsohright, same problem12:53
rubasovOleg commented with one more new idea in the review12:56
rubasovin ovs does a trunk port always (regardless of the value of the trunks attribute) accept untagged frames?12:58
ralonsohrubasov, ok, this is the deal: we can create an interface with an ofport_request12:58
rubasovif not, then Oleg's idea could work12:58
ralonsohwe can have, for example, a "reserved" list of ofports for DHCP ports12:58
ralonsohfrom 64,000 to 65,27912:59
ralonsohthe DHCP agent could create a flow before the port creation, with the ofport know in advance12:59
rubasovthe price seems to be that we have to completely manage the ofport range, because there's no error when the ofport is already taken13:03
rubasovthis gets ofport 10: ovs-vsctl add-port br0 port0 -- set Interface port0 type=internal ofport_request=1013:04
rubasovbut later this throws no error, just silently gets another ofport: ovs-vsctl add-port br0 port1 -- set Interface port1 type=internal ofport_request=1013:04
opendevreviewFernando Royo proposed openstack/ovn-octavia-provider stable/ussuri: Fix race condition retrieving logical router rows  https://review.opendev.org/c/openstack/ovn-octavia-provider/+/82707713:15
rubasovand not just dhcp but each agent plug()-ing has to has its distinct ofport range13:17
*** dasm|off is now known as dasm13:19
*** dasm is now known as dasm|rover13:19
ykarelralonsoh, slaweq when u get chance please check https://review.opendev.org/c/openstack/neutron/+/82705713:20
ykarelneed to clear xena gates13:20
ykarelwould also need re workflow on https://review.opendev.org/c/openstack/neutron/+/826595 as rebased over ^13:21
opendevreviewSlawek Kaplonski proposed openstack/neutron master: Don't filter subnets based on segments if there is no host given  https://review.opendev.org/c/openstack/neutron/+/82710113:58
ralonsohrubasov, ok, last try: how many ports can we have in a single host?14:02
ralonsoh1,000? 5,000? 14:03
rubasovI guess we can introduce new limits if we have to14:03
ralonsohwe have the OVS db locally cached in the IDL connection14:03
rubasovespecially if we make it configurable14:03
ralonsohthat means we can easily read the interface ofports14:03
ralonsohin a single iteration14:03
ralonsohthen we can figure out the free ofports in the system14:04
ralonsohand chose one randomly14:04
ralonsohofport interval:14:04
ralonsohovs-vsctl: constraint violation: -1 is not in the valid range 1 to 65279 (inclusive)14:04
ralonsoh[1, 65279]14:04
ralonsohwe are not going to have more than 5,000 ports14:04
ralonsohthis is impossible to be handled by the OVS agent14:04
ralonsoh(dhcp, router, vms, etc)14:05
rubasovthe previous idea (a configured distinct ofport range for each agent) sounded safer, because if we select randomly, then the dhcp and l3-agent on the same host will start racing and pick the same ofport eventually (the other order will not be atomic either)14:09
ralonsohrubasov, how can we set an ofport range?14:09
ralonsohnow the interface does not define this parameter14:09
ralonsohOVS will chose one but we can't set a limit14:10
ralonsohapart from [1, 65279]14:10
rubasovI'm not saying I like this idea, just for the sake of going through the options: dhcp agent has a configurable range, let's say by default 64000-65000, we pass this down to ovs_lib, ovs_lib keeps track of the range usage, and pre-sets each ports ofport through ofport_request to a value known to be free14:15
rubasovthen the silent ofport change can never happen14:15
rubasovand we can pre-install the flow14:15
ralonsohno, I'm not saying to have a set of ofports for DHCP14:15
ralonsohjust when the port is going to be created list the used ofports and chose a free one14:16
ralonsohwe'll have many chances to use a free one that won't be used in this small period of time when we are creating this port14:16
ralonsohof course, we can check if after the port creation14:17
rubasovbut the "checking which ofport is free and take one" operation is not atomic therefore multiple agents plugging into the same ovs on the same host will race14:17
ralonsohI know14:17
ralonsohthat's why I'm playing with the idea of "there will be only a little change of have this clash"14:18
ralonsohbecause we first read, then create the OF rule and then create the port+interface14:18
ralonsohduring this time I know there is a change for other agent to create another interface with the same ofport14:18
ralonsohchance*14:18
rubasovbut have you considered the birthday problem, I'm not sure if that chance is as low as you would like14:20
rubasovand when it happens, it will be absolutely undebuggable14:20
opendevreviewFernando Royo proposed openstack/ovn-octavia-provider stable/ussuri: Fix race condition retrieving logical router rows  https://review.opendev.org/c/openstack/ovn-octavia-provider/+/82707714:21
opendevreviewLuis Tomas Bolivar proposed openstack/neutron stable/wallaby: Ensure subports status is aligned with parent port  https://review.opendev.org/c/openstack/neutron/+/82691914:27
obondarevrubasov: Hi! please check my last comment in https://review.opendev.org/c/openstack/neutron/+/826460 - I think flow counter growing is ok. and next OVS would actually process the NORMAL action, where the packet will be dropped14:32
opendevreviewSlawek Kaplonski proposed openstack/neutron master: WIP Send packets to the QoS queue using OpenFlow rules  https://review.opendev.org/c/openstack/neutron/+/82711214:42
rubasovsorry guys, I have to take care of something else, I'll be away for about an hour14:43
opendevreviewMerged openstack/neutron stable/xena: [Xena Only] Switch to xena neutron-tempest-plugin jobs  https://review.opendev.org/c/openstack/neutron/+/82705715:22
opendevreviewMerged openstack/neutron stable/xena: [Stable Only] Enforce policy for qos_policy_id attribute  https://review.opendev.org/c/openstack/neutron/+/82659515:22
opendevreviewMerged openstack/neutron stable/xena: [OVN] Add the VIF details "connectivity" parameter  https://review.opendev.org/c/openstack/neutron/+/82643815:23
opendevreviewKrzysztof Tomaszewski proposed openstack/neutron master: Don't set HA ports down while L2 agent restart.  https://review.opendev.org/c/openstack/neutron/+/82654515:27
opendevreviewyatin proposed openstack/neutron stable/wallaby: [Stable Only] Enforce policy for qos_policy_id attribute  https://review.opendev.org/c/openstack/neutron/+/82701515:54
opendevreviewyatin proposed openstack/neutron stable/victoria: [Stable Only] Enforce policy for qos_policy_id attribute  https://review.opendev.org/c/openstack/neutron/+/82701615:54
opendevreviewyatin proposed openstack/neutron stable/ussuri: [Stable Only] Enforce policy for qos_policy_id attribute  https://review.opendev.org/c/openstack/neutron/+/82701715:55
opendevreviewMerged openstack/neutron stable/wallaby: [OVN] Add the VIF details "connectivity" parameter  https://review.opendev.org/c/openstack/neutron/+/82643915:57
fricklerguess you could consider stable/victoria and older gates broken with pip 22 https://zuul.opendev.org/t/openstack/build/5749f117869f4ded9961017509c4b0f216:05
ralonsohfrickler, should we limit the pip version in stable releases?16:10
ralonsohfrickler, is there a bug in devstack open?16:11
fricklerralonsoh: it seems the main thing to do is use a specific version of get-pip.py. and I don't think there's a bug yet, feel free to create one16:16
ralonsohthanks16:16
slaweqralonsoh: mlavalle hi, when You will have some time, please check https://review.opendev.org/c/openstack/neutron/+/827101 which should help to solve issue with Tripleo CI jobs16:39
ralonsohsure16:39
mlavalleslaweq: will do16:40
slaweqwith this patch it seems that tripleo is green again: https://logserver.rdoproject.org/82/38582/4/check/periodic-tripleo-ci-centos-9-ovb-1ctlr_1comp-featureset002-master/ae3536a/16:40
slaweqthx a lot16:40
ralonsohslaweq, actually, when are we passing host?16:46
ralonsohI see no call to this method using "host"16:46
slaweqralonsoh: TBH I didn't check all calls of that method :)16:46
slaweqbut that filtering if there is no host given doesn't makes any sense IMO16:47
ralonsohslaweq, let me check that method and compare it to the older one16:47
slaweqsure16:47
slaweqI will not address any comments there today 16:48
slaweqbut tomorrow morning I would like to move on with this as this is "hot topic" in our cix call :)16:48
opendevreviewFrode Nordahl proposed openstack/neutron master: [OVN] Extend port binding parameter validation  https://review.opendev.org/c/openstack/neutron/+/81842016:48
opendevreviewFrode Nordahl proposed openstack/neutron master: WIP ovn: Off-path SmartNIC DPU Port Binding with OVN  https://review.opendev.org/c/openstack/neutron/+/80896116:48
ralonsohslaweq, lajoskatona frickler https://review.opendev.org/c/openstack/devstack/+/82715516:51
lajoskatonaralonsoh: wasn't this workarounded from some infra repo?16:54
fricklerralonsoh: you've used py syntax, not bash. and I think you need to make some more effort not to use the wrong pre-cached get-pip.py16:54
ralonsohlajoskatona, I wasn't aware of it16:55
ralonsohfrickler, sorry, I don't understand 16:55
fricklerralonsoh: I'll add a comment in the review16:56
ralonsohthanks16:56
opendevreviewLuis Tomas Bolivar proposed openstack/neutron stable/wallaby: Use Port_Binding up column to set Neutron port status  https://review.opendev.org/c/openstack/neutron/+/82715616:56
lajoskatonaralonsoh: I don't know, but there was a mail about it, so I guessed :-)16:57
fricklerlajoskatona: ralonsoh: there are/were further issues that were dealt with on the infra-side, but some fix in devstack is also needed16:58
lajoskatonafrickler: ack16:59
opendevreviewLuis Tomas Bolivar proposed openstack/neutron stable/victoria: Use Port_Binding up column to set Neutron port status  https://review.opendev.org/c/openstack/neutron/+/82327117:07
opendevreviewLuis Tomas Bolivar proposed openstack/neutron stable/ussuri: Use Port_Binding up column to set Neutron port status  https://review.opendev.org/c/openstack/neutron/+/82327217:10
opendevreviewArnau Verdaguer proposed openstack/neutron master: [OVN] Check if exists trunk ports before cleanup  https://review.opendev.org/c/openstack/neutron/+/82715817:18
opendevreviewLuis Tomas Bolivar proposed openstack/networking-ovn stable/train: Use Port_Binding up column to set Neutron port status  https://review.opendev.org/c/openstack/networking-ovn/+/82327917:32
opendevreviewRodolfo Alonso proposed openstack/neutron master: [OVS] Add IPv6 ICMP RA to the default ingress rules  https://review.opendev.org/c/openstack/neutron/+/82715917:36
opendevreviewLajos Katona proposed openstack/neutron master: OVN TestOVNFunctionalBase add waiter for NB_Global  https://review.opendev.org/c/openstack/neutron/+/82553017:37
*** tkajinam is now known as Guest130718:43
opendevreviewFrode Nordahl proposed openstack/neutron master: WIP ovn: Off-path SmartNIC DPU Port Binding with OVN  https://review.opendev.org/c/openstack/neutron/+/80896119:01
opendevreviewMerged openstack/neutron master: Use elevated context to update router's external gateway  https://review.opendev.org/c/openstack/neutron/+/82682820:43
opendevreviewMerged openstack/neutron stable/xena: When creating a VXLAN interface, a device is mandatory  https://review.opendev.org/c/openstack/neutron/+/82475222:12
*** dasm|rover is now known as dasm|off22:29

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!