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

Add support for fabric monitor daemon (swss part). #2920

Merged

Conversation

jfeng-arista
Copy link
Contributor

@jfeng-arista jfeng-arista commented Oct 2, 2023

Add support for fabric monitor daemon ( swss related changes )

Added fabricmgr that reacts the change in config_db and writes to appl_db, as orchagent can only reacts to the appl_db changes in current infrastructure.

So, when users configure the fabric link monitoring polling period or threshold to a none default values, the cli changes the value in config_db. This fabricmgr will react to the change in config_db and copy the value to appl_db. The fabric orchagent can react to the appl_db change and adjust the algorithm accordingly.

need to merge sonic-net/sonic-sairedis#1301 and sonic-net/sonic-buildimage#16791 first for test.

What I did

Why I did it

How I verified it

Details if related

@jfeng-arista
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfeng-arista
Copy link
Contributor Author

this depends on sonic-net/sonic-swss-common#808 . we need submodule update .

@prsunny
Copy link
Collaborator

prsunny commented Oct 2, 2023

@arlakshm , please review

@prsunny prsunny requested a review from arlakshm October 2, 2023 21:20
@prsunny
Copy link
Collaborator

prsunny commented Oct 2, 2023

Please add unit-tests

@jfeng-arista
Copy link
Contributor Author

sonic-net/sonic-sairedis#1301 is the preparation for adding tests for this PR

@jfeng-arista
Copy link
Contributor Author

For running the test, we need to merge
sonic-net/sonic-sairedis#1301 and
sonic-net/sonic-buildimage#16791

kcudnik pushed a commit to sonic-net/sonic-sairedis that referenced this pull request Oct 8, 2023
Add fabric ports related calls for vs tests.

This is the preparation for adding vs test for fabric daemon ( sonic-net/sonic-swss#2920 )
@jfeng-arista jfeng-arista force-pushed the master-add-fabric-monitor-daemon branch from 4a08249 to 7b73c97 Compare October 9, 2023 16:28
@jfeng-arista
Copy link
Contributor Author

the vs test failures needs
sonic-net/sonic-sairedis#1301
and
sonic-net/sonic-buildimage#16791
merge first

arlakshm
arlakshm previously approved these changes Oct 24, 2023
@prsunny
Copy link
Collaborator

prsunny commented Oct 24, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfeng-arista
Copy link
Contributor Author

sonic-net/sonic-sairedis#1301 merged in on Sun, Oct 8
now we need to wait sonic-net/sonic-buildimage#16791   to run the stest.
 

lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Nov 21, 2023
… vs environment (#16791)

Add fabric port data for vs test, and start fabricmgrd in vs environment.

This PR depends on sonic-net/sonic-sairedis#1301

sonic-net/sonic-swss#2920 needs this one merge first.
@arlakshm
Copy link
Contributor

/Azp run Azure.sonic-swss

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arlakshm
Copy link
Contributor

Hi @jfeng-arista sonic-net/sonic-buildimage#16791 is merged now. Can you help update the branch.

@saksarav-nokia
Copy link
Contributor

@jfeng-arista , I am adding the fabric monitoring device files for nokia platform and i am trying to understand the lanes field in the fabric_port_config.ini file. Does this need to match to the SAI_PORT_ATTR_HW_LANE_LIST returned by SAI for the fabric port?.
Could please confirm?

name lanes isolateStatus

Fabric0 0 False
Fabric1 1 False
Fabric2 2 False
Fabric3 3 False
Fabric4 4 False
Fabric5 5 False

Thanks,
Sakthi

@jfeng-arista
Copy link
Contributor Author

test_drop_counters.py:762:


self = <test_drop_counters.TestDropCounters object at 0x7f3a31c208e0>
stats = ['SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_0_DROPPED_PKTS', 'SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS']
counter_list_type = 'PORT_DEBUG_COUNTER_ID_LIST'

def checkFlexState(self, stats, counter_list_type):
    flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE)

    for oid in flex_counter_table.getKeys():
        attributes = self.genericGetAndAssert(flex_counter_table, oid)
        assert len(attributes) == 1
        field, tracked_stats = attributes[0]
        assert field == counter_list_type
        for stat in stats:
          assert stat in tracked_stats

E AssertionError: assert 'SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS' in 'SAI_PORT_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS,SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_0_DROPPED_PKTS'

test_drop_counters.py:109: AssertionError

this is the new test failure, not seems related to this change, will try trigger the test pipeline again

@jfeng-arista
Copy link
Contributor Author

test_drop_counters.py:762:


self = <test_drop_counters.TestDropCounters object at 0x7fd196394a00>
stats = ['SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_0_DROPPED_PKTS', 'SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS']
counter_list_type = 'PORT_DEBUG_COUNTER_ID_LIST'

def checkFlexState(self, stats, counter_list_type):
    flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE)

    for oid in flex_counter_table.getKeys():
        attributes = self.genericGetAndAssert(flex_counter_table, oid)
        assert len(attributes) == 1
        field, tracked_stats = attributes[0]
        assert field == counter_list_type
        for stat in stats:
          assert stat in tracked_stats

E AssertionError: assert 'SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS' in 'SAI_PORT_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS,SAI_PORT_STAT_IN_CONFIGURED_DROP_REASONS_0_DROPPED_PKTS'

test_drop_counters.py:109: AssertionError

@jfeng-arista
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfeng-arista
Copy link
Contributor Author

Now all tests passed

@arlakshm
Copy link
Contributor

arlakshm commented Dec 1, 2023

@prsunny, please help merge this PR

@prsunny
Copy link
Collaborator

prsunny commented Dec 1, 2023

@jfeng-arista , @arlakshm , could you please confirm all dependent PRs are merged and this is ready for merge?

@jfeng-arista
Copy link
Contributor Author

@jfeng-arista , @arlakshm , could you please confirm all dependent PRs are merge and this is ready for merge?

Yes. thank you

@prsunny prsunny merged commit d839eec into sonic-net:master Dec 2, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants