Hi, https://gerrit.fd.io/r/c/vpp/+/37695
Here you go: FEATURE_ALREADY_DISABLED FEATURE_ALREADY_ENABLED There is much work to be done on the error tracking side of the whole nat api though. I wouldn't rely to much on specific errors more on failure / success for now. Best regards, Filip Varga st 23. 11. 2022 o 13:59 filvarga via lists.fd.io <filipvarga89= gmail....@lists.fd.io> napísal(a): > Hi, > > I understand your perspective. But you have to think in a longer > perspective of NAT changes. The concept of enabling / disabling plugins on > the go is not super common among VPP. This was just an addition for SDN > deployments so configuration through startup config can be avoided and > reallocation - change in number of sessions etc. can be done on the fly. > Because of that we haven't yet committed to a "proper" way to handle > events. Why did I choose those return values ? Because UNSUPPORTED is > widely used if some action is not supported at the moment. This also > applies to calling different control plane logic through API on nat44-ed > and among other plugins. So trying to enable already enabled plugin returns > UNSUPPORTED and trying to disable plugin that is not configured will result > in VNET_API_ERROR_UNSUPPORTED. Maybe addition to add > VNET_API_ERROR_FEATURE_ENABLED could work for you. What I would advise is > to wait for this patch to get merged and then add additional > VNET_API_ERROR_FEATURE_ENABLED into vnet/error.h and changing the return > value (just so we comply with how patches should be submitted). > > I will do the same for EI. Thank you for pointing that out. > > Also consider that the part of code returning VNET_API_ERROR_UNSUPPORTED > in nat44-ed enable/disable call for unsupported flags is just a backward > API compatibility logic. That means it should never happen so relying on > VNET_API_ERROR_UNSUPPORTED if plugin is already enabled should be > sufficient. > > As I mentioned before NAT44-ED code architecture isn't ideal but that is > because of its history and both ED and EI and other flavors of NAT where > one big plugin previously (SNAT) now we are moving forward and cleaning it > up (or I am). > > Thank you for your interest. > > Best regards, > Filip Varga > > > st 23. 11. 2022 o 12:59 Jon Loeliger via lists.fd.io <jdl= > netgate....@lists.fd.io> napísal(a): > >> On Wed, Nov 23, 2022 at 12:25 PM filvarga <filipvarg...@gmail.com> wrote: >> >>> Hi, >>> >>> Thank you for your notes and pointing out the issue. Feel free to submit >>> any changes in the future and add appropriate people to your review (based >>> on maintainers list). >>> >> >> Thanks, will do! >> >> For now I am posting a quick fix. As the nat44-ed control plane is >>> undergoing a continuous huge refactor I wouldn't advise too much diverting >>> from its current state.\ >>> >> >> I might point out that this applies to both ED and EI. They both display >> the same issues. >> >> >>> Better solution here is to check the return value of enable / disable >>> calls and then set rv appropriately. >>> >> >> Exactly. But in this case I couldn't because it was buried in a macro >> that didn't return. >> >> >>> For now (based on return codes from vnet/error.h) I will use these >>> VNET_API_ERROR_FEATURE_DISABLED >>> VNET_API_ERROR_UNSUPPORTED >>> for enable/disable api errors rather than introducing something new that >>> can get removed in near future. >>> >> >> Those error values miss the important case of identifying the case where >> one is >> trying to enable the already-enabled feature. We need to know and >> identify that >> case, just as many of the other Features do. >> >> Thanks, >> jdl >> >> >> >> >> > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#22244): https://lists.fd.io/g/vpp-dev/message/22244 Mute This Topic: https://lists.fd.io/mt/95222842/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/leave/1480452/21656/631435203/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
