Skip to content

Commit

Permalink
Fix to not miss the entire set of counters to be added in addObject f…
Browse files Browse the repository at this point in the history
…or CounterContext::updateSupportedCount (#1503)

Fixes issue: sonic-net/sonic-buildimage#21232

In MACSEC there are two set of counters one for INGRESS another for EGRESS which gets mapped to the same COUNTER type - CounterType::MACSEC_SA ( https://github.com/sonic-net/sonic-swss/blob/c20902f3195b5bf8a941045e131aa1b863b69fd0/orchagent/macsecorch.cpp#L2145 )

In the releases after 202205 we started seeing this behavior where the MACSEC RX counters were missing as mentioned in the issue 21232. Further debugging pointed to issue seen after this PR : #1073 was merged.

In this case when macsec orch tries to addCounter for INGRESS SA after the EGRESS SA, and it don't go through as the m_supportedCounters is not empty.

For a fix I am removing the check in CounterContext::updateSupportedCount, which I think is ok as we anyways do a check later on in getSupportedCounters() API using isCounterSupported() before calling collectData() https://github.com/sonic-net/sonic-sairedis/blob/1684aecf7fcc0ecd01aab9bbef855ac483ae2b62/syncd/FlexCounter.cpp#L939

After the fix the macsec SA ingress counters are seen
```
 --------------------------------------- ----------------------------------------------------------------
MACsec port(Ethernet216)
--------------------- ---------------
cipher_suite GCM-AES-XPN-256
enable true
enable_encrypt true
enable_protect true
enable_replay_protect false
profile MACSEC_PROFILE
replay_window 0
send_sci true
--------------------- ---------------
 MACsec Egress SC (xxx)
 ----------- -
 encoding_an 0
 ----------- -
 MACsec Egress SA (0)
 ------------------------------------- ----------------------------------------------------------------
 auth_key xxx
 next_pn 1
 sak xxx
 salt xxx
 ssci 1
 SAI_MACSEC_SA_ATTR_CURRENT_XPN 99
 SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED 17947
 SAI_MACSEC_SA_STAT_OCTETS_PROTECTED 0
 SAI_MACSEC_SA_STAT_OUT_PKTS_ENCRYPTED 98
 SAI_MACSEC_SA_STAT_OUT_PKTS_PROTECTED 0
 ------------------------------------- ----------------------------------------------------------------
 MACsec Ingress SC (xxx)
 MACsec Ingress SA (0)
 --------------------------------------- ----------------------------------------------------------------
 active true
 auth_key xxx
 lowest_acceptable_pn 1
 sak xxx
 salt xxx
 ssci 2
 SAI_MACSEC_SA_ATTR_CURRENT_XPN 203
 SAI_MACSEC_SA_STAT_IN_PKTS_DELAYED 0 <<<<<<<<<<<<<<<<<<<<
 SAI_MACSEC_SA_STAT_IN_PKTS_INVALID 0
 SAI_MACSEC_SA_STAT_IN_PKTS_LATE 0
 SAI_MACSEC_SA_STAT_IN_PKTS_NOT_USING_SA 0
 SAI_MACSEC_SA_STAT_IN_PKTS_NOT_VALID 0
 SAI_MACSEC_SA_STAT_IN_PKTS_OK 5
 SAI_MACSEC_SA_STAT_IN_PKTS_UNCHECKED 0
 SAI_MACSEC_SA_STAT_IN_PKTS_UNUSED_SA 0
 SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED 512
 SAI_MACSEC_SA_STAT_OCTETS_PROTECTED 0 <<<<<<<<<<<<<<<<<<<<<
 --------------------------------------- ----------------------------------------------------------------
```
  • Loading branch information
mssonicbld authored Jan 25, 2025
1 parent e7dc0d2 commit cfd46c4
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
4 changes: 3 additions & 1 deletion syncd/FlexCounter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ class CounterContext : public BaseCounterContext
return;
}

if (always_check_supported_counters)
if (always_check_supported_counters && !dont_clear_support_counter)
{
m_supportedCounters.clear();
}
Expand Down Expand Up @@ -1279,7 +1279,9 @@ std::shared_ptr<BaseCounterContext> FlexCounter::createCounterContext(
else if (context_name == COUNTER_TYPE_MACSEC_SA)
{
auto context = std::make_shared<CounterContext<sai_macsec_sa_stat_t>>(context_name, SAI_OBJECT_TYPE_MACSEC_SA, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
context->use_sai_stats_capa_query = false;
context->dont_clear_support_counter = true;
return context;
}
else if (context_name == COUNTER_TYPE_FLOW)
Expand Down
1 change: 1 addition & 0 deletions syncd/FlexCounter.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ namespace syncd
bool use_sai_stats_ext = false;
bool double_confirm_supported_counters = false;
bool no_double_check_bulk_capability = false;
bool dont_clear_support_counter = false;
};
class FlexCounter
{
Expand Down

0 comments on commit cfd46c4

Please sign in to comment.