| opendevreview | ZhouHeng proposed openstack/neutron master: [ovn]Rescheduler router gateway chassis https://review.opendev.org/c/openstack/neutron/+/825073 | 02:46 |
|---|---|---|
| opendevreview | Merged openstack/neutron master: change skydive source to skydive-project https://review.opendev.org/c/openstack/neutron/+/830114 | 05:05 |
| ralonsoh | lajoskatona, slaweq if you have time: three upper reviews https://review.opendev.org/q/project:openstack%252Fpython-neutronclient+status:open | 06:29 |
| ralonsoh | in particular https://review.opendev.org/c/openstack/python-neutronclient/+/831777 | 06:29 |
| ralonsoh | that is fixing the CI error | 06:30 |
| opendevreview | yatin proposed openstack/neutron-tempest-plugin master: Add script which configures patch ports between bridges https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/763628 | 07:15 |
| lajoskatona | ralonsoh: I will check it | 07:22 |
| gibi | ralonsoh: good morning! Friday I tried to make the MAC overwrite in the port OVO but the GET /ports/<port_id> calls are not going through the OVO. https://review.opendev.org/c/openstack/neutron/+/829247/3#message-823d9eedee5177676e7b92fa784e761cd79a2f6b | 07:49 |
| *** zhouhenglc__ is now known as zhouhenglc | 08:30 | |
| *** gthiemon1e is now known as gthiemonge | 08:39 | |
| opendevreview | Lajos Katona proposed openstack/neutron stable/queens: Fix local doc builds https://review.opendev.org/c/openstack/neutron/+/831856 | 08:57 |
| ralonsoh | gibi, sorry, I didn't check IRC. Let me push a patch today. | 09:13 |
| gibi | OK | 09:13 |
| gibi | thanks | 09:13 |
| gibi | I am happy to work on these patches so for me a decision (and some pointers) would be enough | 09:20 |
| opendevreview | Hemanth N proposed openstack/neutron stable/victoria: [OVN] Allow schema checks to happen before starting connection https://review.opendev.org/c/openstack/neutron/+/830731 | 09:37 |
| ralonsoh | gibi, to be honest, the code handling the port data is quite old | 09:48 |
| ralonsoh | and it was implemented before the fully adoption of OVOS | 09:48 |
| ralonsoh | the creation/update methods depend on the DB object, not the OVO | 09:48 |
| ralonsoh | this is why we have so many *_to_dict methods | 09:49 |
| gibi | ralonsoh: OK, so what would be the way forward? | 09:53 |
| ralonsoh | gibi, let me check | 09:53 |
| gibi | OK | 09:53 |
| gibi | thanks | 09:53 |
| ralonsoh | gibi, the official (and blessed) way to do this is an ML2 extension, same as for port NUMA affinity or port device profile | 10:03 |
| ralonsoh | in this case you don't need to modify any DB model or make any DB change | 10:03 |
| ralonsoh | example | 10:03 |
| ralonsoh | https://review.opendev.org/c/openstack/neutron/+/740067 | 10:03 |
| ralonsoh | with an extension, you create a hook the will be processed with the rest of extensions | 10:04 |
| ralonsoh | you can modify the port dictionary object that is built | 10:04 |
| ralonsoh | for example: https://review.opendev.org/c/openstack/neutron/+/740067/28/neutron/plugins/ml2/extensions/port_numa_affinity_policy.py | 10:04 |
| gibi | what about the original concern that if only the dict is changed then other parts of neutrons that are using the port OVO will not see the changed MAC? | 10:05 |
| ralonsoh | you need to implement only "extend_port_dict" | 10:05 |
| ralonsoh | gibi, that's the point, in the driver's meeting we decided that the original mac was not going to be modified | 10:05 |
| ralonsoh | and with this extension you can update the MAC of the dictionary in a similar way you are modifying it in your patch | 10:06 |
| ralonsoh | but using the "Neutron's official way" | 10:06 |
| gibi | and this way other code in neutron reading the port via the OVO will also see the new MAC from the profile? | 10:06 |
| ralonsoh | gibi, not the OVO but the dictionary that is built from the DB/OVO | 10:07 |
| gibi | but then you mean it is not a concern that other parts of neutron using the port OVO will not see thins change. Is it so? | 10:08 |
| * gibi is confused | 10:08 | |
| ralonsoh | I know | 10:08 |
| ralonsoh | the problem is that many parts of the code doesn't use the OVO | 10:08 |
| ralonsoh | but the ML2 method that uses the DB object | 10:08 |
| ralonsoh | let me check where the OVO is used | 10:09 |
| gibi | OK | 10:09 |
| gibi | so if the ovo is used then we need to change both paths the dict and the OVO path. OR we revisit the idea to overwrite the port.mac_address in the DB | 10:10 |
| ralonsoh | right | 10:10 |
| ralonsoh | gibi, I think we use only use the extension. The new MAC is consumed only by Nova and makes sense that we make this extension discoverable (we don't have micro versioning) | 10:15 |
| ralonsoh | we should* | 10:15 |
| gibi | if we make this as an extension, does it mean we cannot backport his fix to stable branches? | 10:16 |
| ralonsoh | bcafarel, ^^ | 10:17 |
| ralonsoh | could we? | 10:17 |
| gibi | "MAC is conusmed only by Nova" <- what I would like to fix is that the user who looks at the port on the API sees a MAC that is in sync with the MAC in the guest. so this MAC is not only consumed by Nova but also the end user | 10:18 |
| ralonsoh | ahh sorry, bcafarel is not avaiable this week | 10:18 |
| ralonsoh | gibi, if you use this extension, all API calls will return the same modified MAC | 10:18 |
| gibi | OK | 10:19 |
| ralonsoh | (it is like a rule in the "magle" table in the postrouting) | 10:19 |
| gibi | OK, so you suggest to introduce this change as an extension to signal the API behavior change | 10:21 |
| ralonsoh | right | 10:22 |
| gibi | do you suggest to only modify the dict code path? or you suggest to modify both the dict and the OVO path? | 10:22 |
| opendevreview | Merged openstack/python-neutronclient master: Skip B105 pep8 error: hardcoded passwords https://review.opendev.org/c/openstack/python-neutronclient/+/831777 | 10:22 |
| ralonsoh | gibi, and because the old code is so messy (DB objects, OVOs, etc) that is the best way to proceed | 10:22 |
| ralonsoh | we don't have a uniform way to retrieve a port register | 10:23 |
| gibi | sorry I don't get it. so with the extension way, don't we need to add a single codepath modifying the dict format or two code paths modifying both the dict format and the OVO? | 10:26 |
| gibi | (or this question does not make sense) | 10:26 |
| opendevreview | Fernando Royo proposed openstack/ovn-octavia-provider master: Retry of logical switch association to the load balancer for large networks https://review.opendev.org/c/openstack/ovn-octavia-provider/+/829126 | 10:26 |
| ralonsoh | gibi, we do add this change | 10:26 |
| ralonsoh | example (one sec) | 10:26 |
| ralonsoh | https://review.opendev.org/c/openstack/neutron/+/740067/28/neutron/plugins/ml2/extensions/port_numa_affinity_policy.py#46 | 10:27 |
| ralonsoh | this is called in the "_make_port_dict" method | 10:27 |
| gibi | OK, so it handles the dict path | 10:27 |
| ralonsoh | that has a extension processor | 10:27 |
| ralonsoh | yes | 10:27 |
| gibi | do we need some code to the OVO path too? | 10:28 |
| ralonsoh | that could break the code because of how we handle the port register | 10:29 |
| ralonsoh | we don't always use the OVO | 10:29 |
| ralonsoh | most of the times we retrieve the DB port model instead | 10:29 |
| ralonsoh | so depending on what you use, the ML2 get_port method (DB object) or the OVO itseld | 10:29 |
| ralonsoh | you'll have two values | 10:29 |
| ralonsoh | I know this is something to be refactored (but is scary) | 10:30 |
| ralonsoh | this is why I suggest only the extension | 10:30 |
| ralonsoh | it is much safer | 10:30 |
| ralonsoh | (some parts of the code are unmodified since 2014) | 10:31 |
| gibi | so you suggest not to change the OVO path as we assume that no neutron internal user of the OVO path depends on this MAC | 10:31 |
| ralonsoh | right | 10:31 |
| gibi | now I got it. sorry probably I'm slow today :) | 10:31 |
| ralonsoh | no no | 10:31 |
| ralonsoh | this part is quite difficult | 10:32 |
| ralonsoh | mostly because we have this messy code | 10:32 |
| gibi | thanks for the explanation. I will document this in the bug report | 10:33 |
| gibi | ralonsoh: what is you oppinion change the value port.mac_address DB field instead of just changing the port dict? That would be future proof if we ever refactor the ML2 port access to use the OVO instead of a direct DB read | 10:42 |
| opendevreview | Merged openstack/ovn-octavia-provider stable/ussuri: Fix race condition retrieving logical router rows https://review.opendev.org/c/openstack/ovn-octavia-provider/+/827077 | 10:42 |
| opendevreview | Merged openstack/ovn-octavia-provider stable/victoria: Fix race condition retrieving logical router rows https://review.opendev.org/c/openstack/ovn-octavia-provider/+/810489 | 10:42 |
| ralonsoh | gibi, we can implement this change, for sure. That won't require any extension but a change in the ML2 methods creating/updating the port | 10:45 |
| ralonsoh | and a new exception to change the port mac address when the port is bound | 10:45 |
| gibi | you mean a new exception if the MAC in the active binding:profile is changed while the port is in bound state? | 10:46 |
| gibi | today neutron already rejects chaning the port.mac_address if the port is bound | 10:47 |
| gibi | but now there will be a new way via the bindig:profile | 10:47 |
| ralonsoh | gibi, right. That will work (I think) if we have allowed address pairs, but should be tested | 10:47 |
| gibi | please expand a bit on the allowed address pairs thing, I'm not familiar about that | 10:48 |
| ralonsoh | with allowed address pairs we define a set of (ip, mac) that will be associated to a port | 10:49 |
| gibi | ohh | 10:49 |
| ralonsoh | that means we'll allows traffic from/to these tuples (ip, mac) | 10:49 |
| gibi | does that work with direct-phyiscal ports? | 10:49 |
| ralonsoh | and, by default, we usually only defined the IP address | 10:49 |
| ralonsoh | gibi, I don't think so | 10:50 |
| ralonsoh | that is usually for ports that allow port security | 10:50 |
| gibi | becase this whole port:bindig.mac_address -> port.mac_address overwrite only need to work with direct-physical ports. the normal, and direct ports are different, there the user set the MAC on the port and nova/neutron sets that mac on the backend of the port | 10:51 |
| ralonsoh | gibi, yes, this is why, IMO, the extension is enough | 10:51 |
| opendevreview | Candido Campos Rivas proposed openstack/neutron-tempest-plugin master: Recover ssh interface config and interface reload https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/831321 | 10:51 |
| gibi | ralonsoh: I would do the DB update but with a vnic_type conditional so I don't think the DB change would add more change to the behavior than the extension changing the dict | 10:54 |
| ralonsoh | gibi, I think we can also consider this change too, if we properly document it. The mac change rejection was obviously implemented because that was something impossible to happen | 10:56 |
| ralonsoh | but, in this case, it is mandatory | 10:56 |
| gibi | does the DB change doable via an extension? | 11:01 |
| ralonsoh | gibi, let me check | 11:04 |
| ralonsoh | gibi, there are no examples of this in the code for networks, subnets or port. Actually we don't pass the port_db object itself but the dict and the API call arguments. However, we have the session context and the method "process_create_port" or "process_update_port" is called inside the transaction where the port DB object is created | 11:19 |
| ralonsoh | thus we can retrieve the port BD (should be in the transaction itself) | 11:20 |
| ralonsoh | and we can modify it | 11:20 |
| gibi | OK, so an extension could do the DB modification | 11:21 |
| gibi | then I more like the DB modification than just the port_dict modification, it creates less confusion for the next developer when comparing API and DB values | 11:21 |
| gibi | i.e. it results in a more consistend system | 11:22 |
| gibi | *consistent | 11:22 |
| ralonsoh | as I said, this is not done in the code but should be possible | 11:22 |
| gibi | OK. I will play with the current patch and try to make the DB update work there | 11:23 |
| gibi | thanks for the help! | 11:23 |
| ralonsoh | yw | 11:23 |
| opendevreview | Merged openstack/neutron stable/xena: Switch to cirros uec image in ovn tempest jobs https://review.opendev.org/c/openstack/neutron/+/831581 | 11:26 |
| opendevreview | Merged openstack/neutron stable/wallaby: Switch to cirros uec image in ovn tempest jobs https://review.opendev.org/c/openstack/neutron/+/831582 | 11:26 |
| opendevreview | Merged openstack/neutron stable/wallaby: Add centos-9 jobs for wallaby zuul layout https://review.opendev.org/c/openstack/neutron/+/831192 | 11:26 |
| opendevreview | Candido Campos Rivas proposed openstack/neutron-tempest-plugin master: Recover ssh interface config and interface reload https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/831321 | 11:54 |
| opendevreview | Bernard Cafarelli proposed openstack/python-neutronclient stable/yoga: Skip B105 pep8 error: hardcoded passwords https://review.opendev.org/c/openstack/python-neutronclient/+/831858 | 12:39 |
| opendevreview | Bernard Cafarelli proposed openstack/python-neutronclient stable/xena: Skip B105 pep8 error: hardcoded passwords https://review.opendev.org/c/openstack/python-neutronclient/+/831859 | 12:40 |
| bcafarel | ralonsoh: partially available, just expect answers to be a bit laggy :) (or laggier than usual) | 12:40 |
| * bcafarel scrolls back | 12:41 | |
| ralonsoh | no rush | 12:41 |
| ralonsoh | bcafarel, that was related to backporting an extension | 12:41 |
| ralonsoh | is it possible? | 12:41 |
| bcafarel | a new extension? if it is to fix a bug, that can be OK - for existing stable users it would not be enabled by default so it would still be same behaviour | 12:42 |
| bcafarel | and for people that need the fix they would see it in reno and update their configuration to start using it | 12:43 |
| ralonsoh | bcafarel, it is not fixing anything but adding a new feature | 12:43 |
| ralonsoh | perfect then | 12:43 |
| gibi | lajoskatona: ^^ | 12:51 |
| gibi | ralonsoh, bcafarel: just to be cleare we are talking about an API extension. are we? | 12:51 |
| ralonsoh | yes | 12:54 |
| gibi | not an ml2 extension | 12:55 |
| gibi | OK | 12:55 |
| gibi | so api extension is backportable, if I can convince the neutron folks that this is a bugfix not a new feature | 12:56 |
| opendevreview | Fernando Royo proposed openstack/ovn-octavia-provider master: Retry of logical switch association to the load balancer for large networks https://review.opendev.org/c/openstack/ovn-octavia-provider/+/829126 | 13:03 |
| lajoskatona | gibi, ralonsoh, bcafarel: I followed it, and ack | 13:09 |
| lajoskatona | gibi, ralonsoh, bcafarel: for backporting extension: it is not automatic for sure, but can be discussed like we did for quota RBAC (https://meetings.opendev.org/meetings/neutron_drivers/2022/neutron_drivers.2022-03-04-14.03.log.html#l-118 ) | 13:10 |
| gibi | so my plan is then to add an API extension, and implement the port.mac_address DB overwrite | 13:11 |
| opendevreview | Fernando Royo proposed openstack/ovn-octavia-provider master: Retry of logical switch association to the load balancer for large networks https://review.opendev.org/c/openstack/ovn-octavia-provider/+/829126 | 13:27 |
| opendevreview | Merged openstack/neutron stable/train: Allow RBAC on Neutron quotas https://review.opendev.org/c/openstack/neutron/+/828565 | 14:10 |
| opendevreview | Bernard Cafarelli proposed openstack/python-neutronclient stable/xena: Dropping lower constraints testing (stable Xena) https://review.opendev.org/c/openstack/python-neutronclient/+/808597 | 14:26 |
| opendevreview | Fernando Royo proposed openstack/ovn-octavia-provider master: Retry of logical switch association to the load balancer for large networks https://review.opendev.org/c/openstack/ovn-octavia-provider/+/829126 | 14:28 |
| opendevreview | Bernard Cafarelli proposed openstack/python-neutronclient stable/yoga: Update .gitreview for stable/yoga https://review.opendev.org/c/openstack/python-neutronclient/+/831720 | 14:29 |
| opendevreview | Bernard Cafarelli proposed openstack/python-neutronclient stable/yoga: Update TOX_CONSTRAINTS_FILE for stable/yoga https://review.opendev.org/c/openstack/python-neutronclient/+/831722 | 14:29 |
| opendevreview | Merged openstack/neutron-lib master: Update master for stable/yoga https://review.opendev.org/c/openstack/neutron-lib/+/831711 | 14:33 |
| opendevreview | Merged openstack/python-neutronclient stable/yoga: Skip B105 pep8 error: hardcoded passwords https://review.opendev.org/c/openstack/python-neutronclient/+/831858 | 14:49 |
| opendevreview | Merged openstack/python-neutronclient stable/yoga: Update .gitreview for stable/yoga https://review.opendev.org/c/openstack/python-neutronclient/+/831720 | 14:50 |
| opendevreview | Merged openstack/python-neutronclient stable/yoga: Update TOX_CONSTRAINTS_FILE for stable/yoga https://review.opendev.org/c/openstack/python-neutronclient/+/831722 | 14:53 |
| opendevreview | Merged openstack/networking-ovn stable/train: [ovn] Prevent stale ports in the OVN database https://review.opendev.org/c/openstack/networking-ovn/+/828801 | 15:03 |
| opendevreview | yatin proposed openstack/neutron master: [DNM] Check ovn ovs experimental https://review.opendev.org/c/openstack/neutron/+/831220 | 15:25 |
| opendevreview | Merged openstack/neutron master: Group execution of SQL functional tests https://review.opendev.org/c/openstack/neutron/+/831409 | 16:01 |
| opendevreview | Merged openstack/neutron stable/queens: Fix local doc builds https://review.opendev.org/c/openstack/neutron/+/831856 | 16:01 |
| opendevreview | Merged openstack/neutron master: Fix configure_for_func_testing script https://review.opendev.org/c/openstack/neutron/+/830949 | 19:42 |
| opendevreview | Pedro Henrique Pereira Martins proposed openstack/neutron-lib master: Add custom range parameters on port_range validator https://review.opendev.org/c/openstack/neutron-lib/+/832338 | 21:14 |
| opendevreview | Candido Campos Rivas proposed openstack/neutron-tempest-plugin master: Recover ssh interface config and interface reload https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/831321 | 22:18 |
| *** dasm is now known as dasm|off | 23:36 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!