Skip to content

[warm-reboot] Add time diff check between BGP v4 and v6 peers#1014

Merged
neethajohn merged 1 commit intosonic-net:masterfrom
neethajohn:bgp_time_check
Jul 23, 2019
Merged

[warm-reboot] Add time diff check between BGP v4 and v6 peers#1014
neethajohn merged 1 commit intosonic-net:masterfrom
neethajohn:bgp_time_check

Conversation

@neethajohn
Copy link
Copy Markdown
Contributor

@neethajohn neethajohn commented Jul 16, 2019

Description of PR

Testcase for the BGP v6 route reconciliation issue seen in test cluster where BGP v4 and v6 establishment time diff was more than 40s. Issue was fixed in sonic-quagga #37

Type of Change

  • [] Bug Fix
  • [] Testbed and Framework(new/improvement)
  • Test case(new/improvement)

How did you verify/test it?

With the changes, ran warm-reboot test on T0 topology and it passed

@neethajohn
Add time diff check between BGP v4 and v6 peers
5f92912 
Signed-off-by: Neetha John <nejo@microsoft.com>
@pavel-shirshov
Copy link
Copy Markdown
Contributor

pavel-shirshov commented Jul 16, 2019 •
edited by neethajohn
Loading

As we discussed before I don't see a reason to have this check
If you want to check that both ipv4 sessions and ipv6 sessions started on time check them separately.
the ipv6 bgp sessions are not connected in any way to ipv4 bgp sessions neither in the code nor in the theory.
So I don't think it's right to check this constraint.
Also. If your ipv4 gives you a bad timing, this test will find no issue with ipv6 code.

NJ> I though everyone was in agreement to add this check for the time being and later use the DUT syslog for more reliability

@neethajohn neethajohn requested a review from lguohan July 16, 2019 19:37
@yxieca
Copy link
Copy Markdown
Collaborator

yxieca commented Jul 16, 2019

@pavel-shirshov I understand your perspective. This test is for short-term. Neetha will add sonic log analyze over time and check BGP establish time. For now, we agreed that we use difference between IPv4 and IPv6 as an indication of failure.

@pavel-shirshov
Copy link
Copy Markdown
Contributor

pavel-shirshov commented Jul 17, 2019

@yxieca We've already have measurement of IPv4 time for bgp sessions from VM side.
I'd better to add the same measurement for IPv6
We already have all numbers for both ipv4 and ipv6. Let's use this numbers?
https://github.com/Azure/sonic-mgmt/blob/master/ansible/roles/test/files/ptftests/arista.py#L258

@neethajohn neethajohn merged commit 355b542 into sonic-net:master Jul 23, 2019
@neethajohn neethajohn deleted the bgp_time_check branch July 23, 2019 23:58
yxieca pushed a commit that referenced this pull request Jul 24, 2019
@neethajohn @yxieca
Add time diff check between BGP v4 and v6 peers (#1014)
6429d36 
Signed-off-by: Neetha John <nejo@microsoft.com>
@stepanblyschak stepanblyschak mentioned this pull request Dec 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lguohan lguohan lguohan approved these changes

@qiluo-msft qiluo-msft Awaiting requested review from qiluo-msft

@pavel-shirshov pavel-shirshov Awaiting requested review from pavel-shirshov

@yxieca yxieca Awaiting requested review from yxieca

Assignees

No one assigned

Labels

Enhancement Included in 201811 branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@neethajohn @pavel-shirshov @yxieca @lguohan