Update NAT conntrack entries from natmgr instead of natorch by AkhileshSamineni · Pull Request #1274 · sonic-net/sonic-swss

AkhileshSamineni · 2020-04-27T15:48:51Z

Approach:

All NAT conntrack entries would be updated with large timeout (432000) value initially (when entry is created) by sending notifcation from NatSync to NatMgr.
All NAT conntrack enrries would be updated with Zero timeout value (when entry is deleted) be sending notification from OA to NatMgr.
Added a 1 day timer in NatMgr to reset the timeout for all conntrack entries to large time (43200) value.
NatOrch caches the time in secs whenever the entry is active, and compares it with specific timeout for every 30 secs when entry is not active.

Code changes:

NatSync:

Added a new timeout notification producer to send notifications to NatMgr.
Send a notification (whenever a new entry to created) to NatMgr to set 43200 time for that conntrack entry.

NatMgr:

While creating conntrack for Static entries, sets the timeout to max value (432000).
Added a new Refresh Timer for 1 day to set the timeout for all Static conntrack entries to max value (432000).
Added a new flush notification consumer for command “sonic-clear nat translations” and handling “Flushing all conntrack entries and added all existing Static conntrack entries” (This is moved from OA code).
Added an another new timeout notification consumer to handle timeout notifications from NatSync and NatOrch.
Setting the timeout for conntrack entries as per received notifications based on entry’s Key.

NatOrch:

Added a new variable for every entry, it caches the current time whenever entry becomes Active.
If entry is not Active, compares the cached time with current time and if it greater than global timeout, sends a age-out (0 timeout) notification to delete the conntrack entry.
Added a new timeout notification producer to send notification whenever an entry needs to be deleted.
Added a new Refresh Timer for 1 day to send SET (432000) timeout notifications to NatMgr for all dynamic entries.

Depends on:
sonic-net/sonic-utilities#892
sonic-net/sonic-buildimage#4596

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

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

lguohan · 2020-04-29T21:18:15Z

need vs test for this feature.

AkhileshSamineni · 2020-04-30T07:08:21Z

need vs test for this feature.
@lguohan No new functionality added here, this PR is to address review comment ( #1125 (comment))

…NiC/blob/master/doc/nat/nat_design_spec.md" Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com

swssconfig/sample/00-copp.config.json

arlakshm · 2020-05-28T01:37:16Z

need vs test for this feature.
@lguohan No new functionality added here, this PR is to address review comment ( #1125 (comment))
@AkhileshSamineni I think we still need to add a unit test to the new logic to update the conntrack timeouts

arlakshm · 2020-05-28T01:51:30Z

orchagent/natorch.cpp

+            SWSS_LOG_ERROR("Update %s NAT entry [ip %s]", natIter->second.nat_type.c_str(), natIter->first.to_string().c_str());
+            std::vector<FieldValueTuple> fvVector;
+            std::string key = natIter->first.to_string();
+            setTimeoutNotifier->send("SET-SINGLE-NAT", key, fvVector);


It looks like the Timeout value is set to max for active entries in queryhitBits(), why do we need to update the timeout here again

The timeout is set to max value (432000) when entries got added in NatMgr code, in queryhitBits() we are deleting the entries which are not active. Here in this section of code (funtion updateAllConntrackEntries() ) called for every 86400 secs (1 day) to set all dynamic active entries conntrack timeout to Max value (432000).

…ifferent PR. Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

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

AkhileshSamineni · 2020-06-08T11:13:26Z

need vs test for this feature.
@lguohan No new functionality added here, this PR is to address review comment ( #1125 (comment))
@AkhileshSamineni I think we still need to add a unit test to the new logic to update the conntrack timeouts

@arlakshm Added a new test case to verify the conntrack entries timeout value.

AkhileshSamineni · 2020-06-09T04:39:41Z

Retest this please.

arlakshm

LGTM.

abdosi · 2020-08-09T17:58:21Z

@AkhileshSamineni Please create PR for 201911. Cherry-pick has conflict

@arlakshm @rlhui

…#1274) * [consutil] Add UT for consutil clear command * Fix test cases Signed-off-by: Jing Kan jika@microsoft.com

…onic-net#1274) * Update NAT conntrack entries from natmgr instead of natorch Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Update NAT conntrack entries from natmgr instead of natorch

b8cd4e6

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

AkhileshSamineni mentioned this pull request Apr 28, 2020

Separated the notifications for "sonic-clear nat translations" and "sonic-clear nat statistics" commands sonic-net/sonic-utilities#892

Merged

Updated the rate-limit value as per the "https://github.com/Azure/SO…

ec60b79

…NiC/blob/master/doc/nat/nat_design_spec.md" Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com

AkhileshSamineni mentioned this pull request May 7, 2020

[NAT][documentation] Default SNAT DNAT miss packet rate value clarification needed sonic-net/SONiC#611

Closed

AkhileshSamineni mentioned this pull request May 14, 2020

Update the conntrack entries timeout to Max value after warmboot sonic-net/sonic-buildimage#4596

Merged

arlakshm reviewed May 28, 2020

View reviewed changes

swssconfig/sample/00-copp.config.json Outdated Show resolved Hide resolved

arlakshm reviewed May 28, 2020

View reviewed changes

AkhileshSamineni added 2 commits May 28, 2020 00:10

Reverted the copp json file changes as to addressing this issue in d…

c058e1a

…ifferent PR. Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

New testcase to verify the conntrack entries timeout

a66b4be

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

arlakshm approved these changes Jul 8, 2020

View reviewed changes

arlakshm merged commit 5ddea37 into sonic-net:master Jul 8, 2020

arlakshm added the request_for_201911_branch label Jul 14, 2020

abdosi removed the request_for_201911_branch label Sep 19, 2020

EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022

[consutil][test] Add unit tests for consutil clear command (sonic-net…

6fabbed

…#1274) * [consutil] Add UT for consutil clear command * Fix test cases Signed-off-by: Jing Kan jika@microsoft.com

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update NAT conntrack entries from natmgr instead of natorch#1274

Update NAT conntrack entries from natmgr instead of natorch#1274
arlakshm merged 4 commits intosonic-net:masterfrom
AkhileshSamineni:update_conntrack_entries

AkhileshSamineni commented Apr 27, 2020 •

edited

Loading

Uh oh!

lguohan commented Apr 29, 2020

Uh oh!

AkhileshSamineni commented Apr 30, 2020

Uh oh!

Uh oh!

arlakshm commented May 28, 2020

Uh oh!

arlakshm May 28, 2020

Uh oh!

AkhileshSamineni Jun 8, 2020

Uh oh!

AkhileshSamineni commented Jun 8, 2020

Uh oh!

AkhileshSamineni commented Jun 9, 2020

Uh oh!

arlakshm left a comment

Uh oh!

abdosi commented Aug 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

AkhileshSamineni commented Apr 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lguohan commented Apr 29, 2020

Uh oh!

AkhileshSamineni commented Apr 30, 2020

Uh oh!

Uh oh!

arlakshm commented May 28, 2020

Uh oh!

arlakshm May 28, 2020

Choose a reason for hiding this comment

Uh oh!

AkhileshSamineni Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

AkhileshSamineni commented Jun 8, 2020

Uh oh!

AkhileshSamineni commented Jun 9, 2020

Uh oh!

arlakshm left a comment

Choose a reason for hiding this comment

Uh oh!

abdosi commented Aug 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AkhileshSamineni commented Apr 27, 2020 •

edited

Loading