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

Adding additional Ether pkts stats ranges for Cisco Platforms #1997

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Adding additional Ether pkts stats ranges for Cisco Platforms #1997

wants to merge 3 commits into from

Conversation

rajendrat
Copy link

Adding additional Ether Pkts in and out stats ranges requirement for Cisco Platforms.

Currently SAI supported attributes are in the range for bigger frames sizes are : 1519-2047, 2048-4095, 4096-9216 and 9217-16383
Cisco platform supported range for bigger frames sizes are : 1519-2500, 2501-9000, 9001-16383.
It requires new set of Attributes to show granular stats for Cisco platforms.

Adding below Attributes:

  • SAI_PORT_STAT_ETHER_IN_PKTS_1519_TO_2500_OCTETS,
  • SAI_PORT_STAT_ETHER_IN_PKTS_2501_TO_9000_OCTETS,
  • SAI_PORT_STAT_ETHER_IN_PKTS_9001_TO_16383_OCTETS,
  • SAI_PORT_STAT_ETHER_OUT_PKTS_1519_TO_2500_OCTETS,
  • SAI_PORT_STAT_ETHER_OUT_PKTS_2501_TO_9000_OCTETS,
  • SAI_PORT_STAT_ETHER_OUT_PKTS_9001_TO_16383_OCTETS

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 17, 2024

you will need to add exception for SAI_PORT_STAT_END in meta/ancestry.pl at line 180

@rajendrat rajendrat force-pushed the rajendrat/AdditionalPortStatsRange branch from 15fb33d to 12bc084 Compare April 17, 2024 18:28
@rajendrat
Copy link
Author

you will need to add exception for SAI_PORT_STAT_END in meta/ancestry.pl at line 180

Thanks @kcudnik. Created exception for SAI_PORT_STAT_END, still i see the meta check failure. Any input?

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 18, 2024

you need to squash your commits

@rajendrat rajendrat force-pushed the rajendrat/AdditionalPortStatsRange branch 3 times, most recently from 86ec786 to 363316d Compare April 18, 2024 17:22
prgeor
prgeor reviewed Apr 18, 2024
View reviewed changes
inc/saiport.h Show resolved Hide resolved
@rajendrat rajendrat force-pushed the rajendrat/AdditionalPortStatsRange branch from 3ae3678 to 6d58942 Compare April 19, 2024 18:01
@rajendrat
Copy link
Author

you need to squash your commits

@kcudnik : Even after squash I see the issue. is there any skip required in CheckHash ?

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 20, 2024

please read pipeline errors:

Updated 71 paths from 1b10e39
Checking for possible enum values shift (current branch vs origin/master) ...
ERROR: value of SAI_PORT_STAT_DOT3_CONTROL_IN_UNKNOWN_OPCODES differ: ../inc//saiport.h:2595 vs temp/inc//saiport.h:2583 => (178 != 172)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_ALIGNMENT_ERRORS differ: ../inc//saiport.h:2569 vs temp/inc//saiport.h:2557 => (165 != 159)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_CARRIER_SENSE_ERRORS differ: ../inc//saiport.h:2587 vs temp/inc//saiport.h:2575 => (174 != 168)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_DEFERRED_TRANSMISSIONS differ: ../inc//saiport.h:2579 vs temp/inc//saiport.h:2567 => (170 != 164)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_EXCESSIVE_COLLISIONS differ: ../inc//saiport.h:2583 vs temp/inc//saiport.h:2571 => (172 != 166)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_FCS_ERRORS differ: ../inc//saiport.h:2571 vs temp/inc//saiport.h:2559 => (166 != 160)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_FRAME_TOO_LONGS differ: ../inc//saiport.h:2589 vs temp/inc//saiport.h:2577 => (175 != 169)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_INTERNAL_MAC_RECEIVE_ERRORS differ: ../inc//saiport.h:2591 vs temp/inc//saiport.h:2579 => (176 != 170)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_INTERNAL_MAC_TRANSMIT_ERRORS differ: ../inc//saiport.h:2585 vs temp/inc//saiport.h:2573 => (173 != 167)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_LATE_COLLISIONS differ: ../inc//saiport.h:2581 vs temp/inc//saiport.h:2569 => (171 != 165)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_MULTIPLE_COLLISION_FRAMES differ: ../inc//saiport.h:2575 vs temp/inc//saiport.h:2563 => (168 != 162)
ERROR: value of SAI_PORT_STAT_DOT3_STATS_SINGLE_COLLISION_FRAMES differ: ../inc//saiport.h:2573 vs temp/inc//saiport.h:2561 => (167 != 161)

you shifted enum values

@rajendrat rajendrat force-pushed the rajendrat/AdditionalPortStatsRange branch 3 times, most recently from 3e6d888 to 26f2dc8 Compare April 22, 2024 23:14
kcudnik
kcudnik reviewed Apr 23, 2024
View reviewed changes
meta/ancestry.pl Outdated
@@ -177,6 +177,7 @@ sub BuildCommitHistory
next if $enumName eq "SAI_API_MAX";
next if $enumName eq "SAI_OBJECT_TYPE_MAX";
next if $enumName eq "SAI_PORT_INTERFACE_TYPE_MAX";
next if $enumName eq "SAI_PORT_STAT_END";
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, interesting that line 174 didnt catch that

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 23, 2024 

Checking for possible enum values shift (current branch vs origin/master) ...
ERROR: value of SAI_PORT_STAT_END differ: ../inc//saiport.h:2726 vs temp/inc//saiport.h:2714 => (12294 != 12288)
enum SAI_PORT_STAT_ETHER_IN_PKTS_1519_TO_2500_OCTETS only defined in ../inc//saiport.h:2714
enum SAI_PORT_STAT_ETHER_IN_PKTS_2501_TO_9000_OCTETS only defined in ../inc//saiport.h:2716
enum SAI_PORT_STAT_ETHER_IN_PKTS_9001_TO_16383_OCTETS only defined in ../inc//saiport.h:2718
enum SAI_PORT_STAT_ETHER_OUT_PKTS_1519_TO_2500_OCTETS only defined in ../inc//saiport.h:2720
enum SAI_PORT_STAT_ETHER_OUT_PKTS_2501_TO_9000_OCTETS only defined in ../inc//saiport.h:2722
enum SAI_PORT_STAT_ETHER_OUT_PKTS_9001_TO_16383_OCTETS only defined in ../inc//saiport.h:2724
ERROR: value of SAI_PORT_STAT_END differ: temp/inc//saiport.h:2714 vs ../inc//saiport.h:2726 => (12288 != 12294)
ERROR: please correct all 2 error(s) before continue

you will need to add exception for this error, you will need to add excetpion for checkheaders.pl on line 196, and that chance in ancestry.pl is not needed

@rajendrat rajendrat force-pushed the rajendrat/AdditionalPortStatsRange branch from 26f2dc8 to 636fdcd Compare April 23, 2024 16:46
@kcudnik
Copy link
Collaborator

kcudnik commented Apr 23, 2024

i think you will also need to rebase to master

@rajendrat
Copy link
Author

i think you will also need to rebase to master

@kcudnik : Tried rebase to master, still seeing the same error.

2024-04-23T16:50:46.5235965Z ERROR: FATAL: sai_attribute_value_t members were removed on commit 636fdcd, NOT ALLOWED!

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 23, 2024

no you didn't:

* 636fdcd (1997) Adding additional Ether pkts stats ranges for Cisco Platforms
| * e329bf2 (origin/master, origin/HEAD) [meta] Fix dangling pointer warning for gcc 12 (#1996)
| * 2587c3b (tag: v1.14.0, origin/v1.14) SAI Proposal for PoE Support (#1977)
|/  
* ca6b08f Update sai version to v1.14.0 (#1991)

@rajendrat
Adding additional Ether pkts stats ranges for Cisco Platforms
927f7e6 
Signed-off-by: rajendrat <rajendrat@gmail.com>
@rajendrat rajendrat force-pushed the rajendrat/AdditionalPortStatsRange branch from 4d199dc to 927f7e6 Compare April 23, 2024 20:54
@rajendrat
Copy link
Author

@kcudnik , @prgeor : All checks passed, could you please merge this PR.

@lguohan
Copy link
Collaborator

lguohan commented Apr 29, 2024

i suggest to have a RO attribute defined to indicate which range the asic support. for example, the traditional range, or this new packet size range.

we can say the RO default value is triditional range. if they asic support new range, then when application queries, it will return the range_2 for example. this allows application to write code with capability.

@rlhui
Copy link
Collaborator

rlhui commented Sep 19, 2024

@rajendrat please resolve conflicts.

@rlhui
Copy link
Collaborator

rlhui commented Sep 19, 2024

i suggest to have a RO attribute defined to indicate which range the asic support. for example, the traditional range, or this new packet size range.

we can say the RO default value is triditional range. if they asic support new range, then when application queries, it will return the range_2 for example. this allows application to write code with capability.

@rajendrat any update on this

@rlhui rlhui added the reviewed PR is discussed in SAI Meeting label Sep 19, 2024
@kcudnik
Copy link
Collaborator

kcudnik commented Oct 2, 2024

you will need to rebase on master, resolve conflicts and squash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prgeor prgeor prgeor approved these changes

@kcudnik kcudnik kcudnik approved these changes

Assignees
No one assigned
Labels
reviewed PR is discussed in SAI Meeting
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rajendrat @kcudnik @lguohan @rlhui @prgeor