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

[bfdorch] add default TOS value for BFD session #2689

Merged
merged 4 commits into from
Mar 7, 2023

Conversation

baorliu
Copy link
Contributor

@baorliu baorliu commented Mar 4, 2023

What I did
Add default TOS value when create BFD session, default DSCP value is 48, default ECN is 0. The BFD session configuration will override this default value.

Why I did it
Higher DSCP value is preferred for BFD packet.

How I verified it
Set different TOS value in BFD session configuration, and create BFD session in DUT.
Load the test image with the design change, connect a traffic generator (with BFD protocol support), capture the packet sent out by DUT, check the DSCP and ECN field in the captured packet.

cisco@sonic:/var/tmp$ cat bfd.cfg
[
  {
      "BFD_SESSION_TABLE:default:default:192.168.88.2": {
      "tos": "186",
      "multihop": "true",
      "rx_interval": "1000",
      "tx_interval": "1000",
      "local_addr": "192.168.88.1"
  },
  "OP": "SET"
}
]

image

APP_DB dump:


cisco@sonic:/var/tmp$ head -n 50 redis0.log 
{
  "BFD_SESSION_TABLE:default:default:192.168.88.2": {
    "expireat": 1677887525.8233933,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "local_addr": "192.168.88.1",
      "multihop": "true",
      "rx_interval": "1000",
      "tos": "186",
      "tx_interval": "1000"
    }
  },

ASIC DB dump:

  "ASIC_STATE:SAI_OBJECT_TYPE_BFD_SESSION:oid:0x45000000000989": {
  "expireat": 1677887399.4435563,
  "ttl": -0.001,
  "type": "hash",
  "value": {
    "SAI_BFD_SESSION_ATTR_BFD_ENCAPSULATION_TYPE": "SAI_BFD_ENCAPSULATION_TYPE_NONE",
    "SAI_BFD_SESSION_ATTR_DST_IP_ADDRESS": "192.168.88.2",
    "SAI_BFD_SESSION_ATTR_IPHDR_VERSION": "4",
    "SAI_BFD_SESSION_ATTR_LOCAL_DISCRIMINATOR": "6",
    "SAI_BFD_SESSION_ATTR_MIN_RX": "1000000",
    "SAI_BFD_SESSION_ATTR_MIN_TX": "1000000",
    "SAI_BFD_SESSION_ATTR_MULTIHOP": "true",
    "SAI_BFD_SESSION_ATTR_MULTIPLIER": "10",
    "SAI_BFD_SESSION_ATTR_REMOTE_DISCRIMINATOR": "0",
    "SAI_BFD_SESSION_ATTR_SRC_IP_ADDRESS": "192.168.88.1",
    "SAI_BFD_SESSION_ATTR_TOS": "186",
    "SAI_BFD_SESSION_ATTR_TYPE": "SAI_BFD_SESSION_TYPE_ASYNC_ACTIVE",
    "SAI_BFD_SESSION_ATTR_UDP_SRC_PORT": "49157",
    "SAI_BFD_SESSION_ATTR_VIRTUAL_ROUTER": "oid:0x3000000000042"
  }
},

Details if related

Add default TOS value when create BFD session, default DSCP value is 46, default ECN is 0. The BFD session configuration will override this default value.
@baorliu
Copy link
Contributor Author

baorliu commented Mar 4, 2023

BFD related UT passed. the vstest failure caused by other UT.

2023-03-04T03:26:50.4079122Z test_addRemoveBfdSession passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4079402Z test_addRemoveBfdSession_ipv6 passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4079694Z test_addRemoveBfdSession_interface passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4079980Z test_addRemoveBfdSession_txrx_interval passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4080285Z test_addRemoveBfdSession_multiplier passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4080582Z test_addRemoveBfdSession_multihop passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4080893Z test_addRemoveBfdSession_type passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4081203Z test_multipleBfdSessions passed 1 out of the required 1 times. Success!
2023-03-04T03:26:50.4081478Z test_bfd_state_db_clear passed 1 out of the required 1 times. Success!

@baorliu baorliu marked this pull request as ready for review March 4, 2023 20:06
@baorliu baorliu requested a review from prsunny as a code owner March 4, 2023 20:06
@lguohan
Copy link
Contributor

lguohan commented Mar 5, 2023

@prsunny , can yu review this?

@@ -243,6 +243,7 @@ bool BfdOrch::create_bfd_session(const string& key, const vector<FieldValueTuple
uint32_t tx_interval = BFD_SESSION_DEFAULT_TX_INTERVAL;
uint32_t rx_interval = BFD_SESSION_DEFAULT_RX_INTERVAL;
uint8_t multiplier = BFD_SESSION_DEFAULT_DETECT_MULTIPLIER;
uint8_t tos = 184; // default value 46<<2. higher 6-bit tos 46, lower 2-bit ecn 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use a # define to declare this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. thanks.

@prsunny prsunny requested a review from siqbal1986 March 6, 2023 22:42
@siqbal1986
Copy link
Contributor

can you please update it to reflect the use of #defile and change the value to 48 as done in #2690

@prsunny
Copy link
Collaborator

prsunny commented Mar 7, 2023

can you please raise a PR to 202012?

@baorliu baorliu force-pushed the cisco-baorliu-bfd-tos_t branch from ec5cedc to c5fd742 Compare March 7, 2023 05:54
Copy link
Contributor

@siqbal1986 siqbal1986 left a comment

Choose a reason for hiding this comment

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

looks good

@siqbal1986
Copy link
Contributor

can you please create a PR for 202012 as well.

@siqbal1986 siqbal1986 merged commit 115efe8 into sonic-net:master Mar 7, 2023
siqbal1986 pushed a commit that referenced this pull request Mar 7, 2023
AntonHryshchuk added a commit to AntonHryshchuk/sonic-buildimage that referenced this pull request Mar 8, 2023
Update sonic-swss submodule pointer to include the following:
* a2c9a61 [EVPN]Handling error scenarios during route programming and IMR add ([sonic-net#2670](sonic-net/sonic-swss#2670))
* 115efe8 [bfdorch] add default TOS value for BFD session ([sonic-net#2689](sonic-net/sonic-swss#2689))
* a198289 [orchagent, SRv6]: create seglist support to set sid list type ([sonic-net#2406](sonic-net/sonic-swss#2406))

Signed-off-by: AntonHryshchuk <antonh@nvidia.com>
yxieca pushed a commit that referenced this pull request Mar 8, 2023
What I did
Add default TOS value when create BFD session, default DSCP value is 46, default ECN is 0. The BFD session configuration will override this default value.

Why I did it
Higher DSCP value is preferred for BFD packet.

How I verified it
Set different TOS value in BFD session configuration, and create BFD session in DUT.
Load the test image with the design change, connect a traffic generator (with BFD protocol support), capture the packet sent out by DUT, check the DSCP and ECN field in the captured packet.
dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Mar 14, 2023
Update sonic-swss submodule pointer to include the following:
* 98a16cf [ACL] Write ACL table/rule creation status into STATE_DB ([sonic-net#2662](sonic-net/sonic-swss#2662))
* a2c9a61 [EVPN]Handling error scenarios during route programming and IMR add ([sonic-net#2670](sonic-net/sonic-swss#2670))
* 115efe8 [bfdorch] add default TOS value for BFD session ([sonic-net#2689](sonic-net/sonic-swss#2689))
* a198289 [orchagent, SRv6]: create seglist support to set sid list type ([sonic-net#2406](sonic-net/sonic-swss#2406))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
prsunny pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Mar 14, 2023
Update sonic-swss submodule pointer to include the following:
* 98a16cf [ACL] Write ACL table/rule creation status into STATE_DB ([#2662](sonic-net/sonic-swss#2662))
* a2c9a61 [EVPN]Handling error scenarios during route programming and IMR add ([#2670](sonic-net/sonic-swss#2670))
* 115efe8 [bfdorch] add default TOS value for BFD session ([#2689](sonic-net/sonic-swss#2689))
* a198289 [orchagent, SRv6]: create seglist support to set sid list type ([#2406](sonic-net/sonic-swss#2406))
StormLiangMS pushed a commit that referenced this pull request Mar 19, 2023
What I did
Add default TOS value when create BFD session, default DSCP value is 46, default ECN is 0. The BFD session configuration will override this default value.

Why I did it
Higher DSCP value is preferred for BFD packet.

How I verified it
Set different TOS value in BFD session configuration, and create BFD session in DUT.
Load the test image with the design change, connect a traffic generator (with BFD protocol support), capture the packet sent out by DUT, check the DSCP and ECN field in the captured packet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants