Re: [vpp-dev] Fixing nat44ed vrf api handlers

2023-03-27 Thread Daniel Béreš
I split it up and it's ready for review: https://gerrit.fd.io/r/c/vpp/+/38459 https://gerrit.fd.io/r/c/vpp/+/38551 I have last one question: In case API doesn't return all data (e.g nat44_ed_vrf_tables_send_details missing  table_vrf_id assign of value) and if I decide to add it. Is it

Re: [vpp-dev] Fixing nat44ed vrf api handlers

2023-03-22 Thread filvarga
If you decide to split the patch and do a second version. I would like to see the first version set as deprecated. So when removed we can put it again in without having to have _v2 _v3 etc. Best regards, Filip Varga st 22. 3. 2023 o 15:33 Daniel Béreš napísal(a): > Thanks for the reasonable

Re: [vpp-dev] Fixing nat44ed vrf api handlers

2023-03-22 Thread Daniel Béreš
Thanks for the reasonable explanations. So the option could be splitting it into two patches. The first could fix replies and the second could contain a new experimental API nat44_ed_vrf_tables_send_details_v2. I think it might be okay after that. Daniel -=-=-=-=-=-=-=-=-=-=-=- Links: You

Re: [vpp-dev] Fixing nat44ed vrf api handlers

2023-03-22 Thread Andrew Yourtchenko
Some more context: after discussion with Ole: I think I might have overstepped the authority in terms of "-2" - it is component maintainers' decision if they want to put even completely breaking change into their component that sets the world on fire - if the CI lets that through - it is their

Re: [vpp-dev] Fixing nat44ed vrf api handlers

2023-03-22 Thread filvarga
Hi, Yes I fully agree with Andrew on changing APIs. Though the api was broken in the first place. If used incorrect message is returned. Because of that I am not even sure if it is functional at all. It is more likely that nobody before addressed this issue because this feature is not used. So I

Re: [vpp-dev] Fixing nat44ed vrf api handlers

2023-03-22 Thread Andrew Yourtchenko
Hey Daniel,I had a quick look and it looks like the APIs in question are also not marked as experimental ?If I am right, then to me seems like a silent behavior change to a production API - something we have heard repeated complaints from the downstream consumers for….I had -2’d it for now - we

[vpp-dev] Fixing nat44ed vrf api handlers

2023-03-22 Thread Daniel Béreš
Hello Guys, I wrote a short patch to fix a couple api handlers for nat44ed [1] . ( https://gerrit.fd.io/r/c/vpp/+/38459 ) It was probably forgotten, so I'm reminding myself. I still have a question about whether it is possible to cherry-pick the patch to vpp v23.02 and 22.10. [1]