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.

Sign up for GitHub

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

Jump to bottom

[TSA]: Use egrep to match TSA rules better #6403

Merged
merged 1 commit into from
Jan 14, 2021
Merged

[TSA]: Use egrep to match TSA rules better #6403

merged 1 commit into from
Jan 14, 2021

Conversation

pavel-shirshov
Copy link
Contributor

@pavel-shirshov pavel-shirshov commented Jan 8, 2021 •
edited
Loading

- Why I did it
Earlier today we found a bug in the SONiC TSA implementation.
TSC shows incorrect output (see below) in case we have a route-map which contains TSA route-map as a prefix.

admin@str-s6100-acs-1:~$ TSC
Traffic Shift Check:
System Mode: Not consistent

The reason is that TSC implementation has too loose regexps in TSA utilities, which match wrong route-map entries:
For example, current TSC matches following

route-map TO_BGP_PEER_V4 permit 200
route-map TO_BGP_PEER_V6 permit 200

But it should match only

route-map TO_BGP_PEER_V4 permit 20
route-map TO_BGP_PEER_V4 deny 30
route-map TO_BGP_PEER_V6 permit 20
route-map TO_BGP_PEER_V6 deny 30

- How I did it
I fixed it by using egrep with ^ and $ regexp markers which match begin and end of the line.

- How to verify it

  1. Add follwing entry to FRR config:
str-s6100-acs-1# 
str-s6100-acs-1# conf t
str-s6100-acs-1(config)# route-map TO_BGP_PEER_V4 permit 200 
str-s6100-acs-1(config-route-map)# end
  1. Use the TSC command and check output. It should show normal.
admin@str-s6100-acs-1:~$ TSC
Traffic Shift Check:
System Mode: Normal```


**- Which release branch to backport (provide reason below if selected)**

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [x] 201911
- [x] 202006
- [x] 202012

**- Description for the changelog**
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->


**- A picture of a cute animal (not mandatory but encouraged)**

@pavel-shirshov pavel-shirshov marked this pull request as ready for review January 8, 2021 22:29
@lguohan
Copy link
Collaborator

lguohan commented Jan 9, 2021

retest vsimage please

4 similar comments
@lguohan
Copy link
Collaborator

lguohan commented Jan 12, 2021

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Jan 12, 2021

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Jan 13, 2021

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Jan 14, 2021

retest vsimage please

@lguohan lguohan merged commit 16e5434 into sonic-net:master Jan 14, 2021
lguohan pushed a commit that referenced this pull request Jan 15, 2021
@pavel-shirshov @lguohan
[docker-frr]: Use egrep with regexp to match correct TSA rules (#6403)
cd8417a 
**- Why I did it**
Earlier today we found a bug in the SONiC TSA implementation.
TSC shows incorrect output (see below) in case we have a route-map which contains TSA route-map as a prefix.
```
admin@str-s6100-acs-1:~$ TSC
Traffic Shift Check:
System Mode: Not consistent
```
The reason is that TSC implementation has too loose regexps in TSA utilities, which match wrong route-map entries:
For example, current TSC matches following
```
route-map TO_BGP_PEER_V4 permit 200
route-map TO_BGP_PEER_V6 permit 200
```
But it should match only
```
route-map TO_BGP_PEER_V4 permit 20
route-map TO_BGP_PEER_V4 deny 30
route-map TO_BGP_PEER_V6 permit 20
route-map TO_BGP_PEER_V6 deny 30
```

**- How I did it**
I fixed it by using egrep with `^` and `$` regexp markers which match begin and end of the line.

**- How to verify it**
1. Add follwing entry to FRR config:
```
str-s6100-acs-1# 
str-s6100-acs-1# conf t
str-s6100-acs-1(config)# route-map TO_BGP_PEER_V4 permit 200 
str-s6100-acs-1(config-route-map)# end
```
2. Use the TSC command and check output. It should show normal.
```
admin@str-s6100-acs-1:~$ TSC
Traffic Shift Check:
System Mode: Normal```
abdosi pushed a commit that referenced this pull request Jan 20, 2021
@pavel-shirshov @abdosi
[docker-frr]: Use egrep with regexp to match correct TSA rules (#6403)
beaaf33 
**- Why I did it**
Earlier today we found a bug in the SONiC TSA implementation.
TSC shows incorrect output (see below) in case we have a route-map which contains TSA route-map as a prefix.
```
admin@str-s6100-acs-1:~$ TSC
Traffic Shift Check:
System Mode: Not consistent
```
The reason is that TSC implementation has too loose regexps in TSA utilities, which match wrong route-map entries:
For example, current TSC matches following
```
route-map TO_BGP_PEER_V4 permit 200
route-map TO_BGP_PEER_V6 permit 200
```
But it should match only
```
route-map TO_BGP_PEER_V4 permit 20
route-map TO_BGP_PEER_V4 deny 30
route-map TO_BGP_PEER_V6 permit 20
route-map TO_BGP_PEER_V6 deny 30
```

**- How I did it**
I fixed it by using egrep with `^` and `$` regexp markers which match begin and end of the line.

**- How to verify it**
1. Add follwing entry to FRR config:
```
str-s6100-acs-1# 
str-s6100-acs-1# conf t
str-s6100-acs-1(config)# route-map TO_BGP_PEER_V4 permit 200 
str-s6100-acs-1(config-route-map)# end
```
2. Use the TSC command and check output. It should show normal.
```
admin@str-s6100-acs-1:~$ TSC
Traffic Shift Check:
System Mode: Normal```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abdosi abdosi abdosi approved these changes

@ivlee1 ivlee1 Awaiting requested review from ivlee1

Assignees
No one assigned
Labels
Included in 201911 Branch Request for 201911 Branch :shipit:
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pavel-shirshov @lguohan @abdosi