Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[201911] Update nat entries to use nat_type to support DNAT Pool changes. #1297

Merged
merged 3 commits into from
Jul 21, 2020

Conversation

AkhileshSamineni
Copy link
Contributor

@AkhileshSamineni AkhileshSamineni commented May 21, 2020

Code changes:

NatMgr:

  1. Setting the "APP_NAT_DNAT_POOL_TABLE_NAME" table with destination-ip when first Static NAT/NAPT entry is added at config_db.
  2. Incrementing the ref count in "m_natDnatPoolInfo" for same destination-ip for next Static NAT/NAPT entry is added at config_db.
  3. Decrementing the reference count in "m_natDnatPoolInfo" for same destination-ip when Static NAT/NAPT entry is deleted at config_db.
  4. Deleting the "APP_NAT_DNAT_POOL_TABLE_NAME" table with destination-ip when last Static NAT/NAPT entry is deleted at config_db.
  5. It does the same for all IP-Addresses from pool-addresses range in NAT Pool configuration in config_db.

NatOrch:

  1. Creating DNAT_POOL entries using nat_type as "SAI_NAT_TYPE_DESTINATION_NAT_POOL" when "APP_NAT_DNAT_POOL_TABLE_NAME" table is created.
  2. Deleting DNAT_POOL entries using nat_type as "SAI_NAT_TYPE_DESTINATION_NAT_POOL" when "APP_NAT_DNAT_POOL_TABLE_NAME" table is deleted.
  3. Updated code to use nat_type while creating the nat entries.

Addressing the issue - #1234

Depends on :
sonic-net/sonic-utilities#921
sonic-net/sonic-sairedis#616
sonic-net/sonic-swss-common#354

Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
@AkhileshSamineni
Copy link
Contributor Author

Retest this please.

@AkhileshSamineni
Copy link
Contributor Author

test this please.


/* Add DnatPool entry to APPL_DB*/
SWSS_LOG_INFO("Adding dnat pool entry for %s", m_staticNaptEntry[key].local_ip.c_str());
addDnatPoolEntry(m_staticNaptEntry[key].local_ip);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need DNAT pool entry for the local_ip? I believe DNAT pool lookup is required only for the translated global ip before DNAT/DNAPT table lookup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User can configure the local IP as the translated source IP in the SNAT direction. Hence in the reverse DNAT direction, we need the DNAT Pool entry for the local IP.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kperumalbfn, wondering if this PR looks OK? Thanks.

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
@AkhileshSamineni
Copy link
Contributor Author

@rlhui @kperumalbfn Compilation is failing with below error which is unrelated to our PR changes, not sure what could be reason for this failure.

[2020-05-31 02:02:27] [build] fpmsyncd.cpp:3:10: fatal error: logger.h: No such file or directory
[2020-05-31 02:02:27] [build] 3 | #include "logger.h"
[2020-05-31 02:02:27] [build] | ^~~~~~~~~~
[2020-05-31 02:02:27] [build] compilation terminated.

I see some other PR (#1305) also failing with same error.

@kperumalbfn
Copy link
Contributor

@rlhui @kperumalbfn Compilation is failing with below error which is unrelated to our PR changes, not sure what could be reason for this failure.

[2020-05-31 02:02:27] [build] fpmsyncd.cpp:3:10: fatal error: logger.h: No such file or directory
[2020-05-31 02:02:27] [build] 3 | #include "logger.h"
[2020-05-31 02:02:27] [build] | ^~~~~~~~~~
[2020-05-31 02:02:27] [build] compilation terminated.

I see some other PR (#1305) also failing with same error.

@rlhui Any known issue with "logger.h" include file? Can we restart the sanity??

@rlhui
Copy link
Contributor

rlhui commented Jun 25, 2020

Retest this please

@arlakshm
Copy link
Contributor

retest this please

@rlhui
Copy link
Contributor

rlhui commented Jul 16, 2020

@AkhileshSamineni , please check LGTM failures, thanks.

@kperumalbfn
Copy link
Contributor

@daall LGTM errors on 201911 branch. Is this known one as per your comment on other PR #1355?? If so, can we merge this?

@daall
Copy link
Contributor

daall commented Jul 20, 2020

@kperumalbfn you are right that it is the same issue, @rlhui you can ignore the LGTM result for this PR as it is not properly set up to build with 201911.

@AkhileshSamineni AkhileshSamineni changed the title Update nat entries to use nat_type to support DNAT Pool changes. [201911] Update nat entries to use nat_type to support DNAT Pool changes. Jul 21, 2020
@rlhui rlhui merged commit 51e0e17 into sonic-net:201911 Jul 21, 2020
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
Remove redundant 'pcie-' prefix from subcommands

Note this will also require an update to files/image_config/pcie-check/pcie-check.sh in sonic-buildimage. Also, since these commands require sudo, they need to be added to the read-only commands in the sudoers file, which they have not yet been.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants