Rename ENI attribute PPS to BW to reflect the actual BW limitation in bytes #526
Conversation
|
heads to up @prsunny @theasianpianist , this will cause one of the existing SAI attribute being changed. but it should be aligned with our current HLD. |
| @@ -36,7 +36,7 @@ enum bit<16> dash_tunnel_dscp_mode_t { | |||
|
|
|||
| struct eni_data_t { | |||
| bit<32> cps; | |||
| bit<32> pps; | |||
| bit<32> bw; | |||
There was a problem hiding this comment.
although bw is the one we use in the HLD, I wonder if we like to get the unit clear, such as bw_in_kbps.
There was a problem hiding this comment.
@r12f good idea. At least PPS is unambiguous. The item name should include the units to make it clear.
There was a problem hiding this comment.
All BW throughout SAI is in Mb/s
There was a problem hiding this comment.
why not keep it bw and bw can accept values like 978, 1K, 1M, 15G, 2T, (if we don't want to type too many zeroes) and the last char if letter detonates the unit this way we don't end up with 20 api names, like bw_in_3_and_a_half_bytes_increments (exaggeration to make the point),
take it further and in the value you can add last 2 letters as 1000Kb or 1000KB, or have convention that uppercase means bytes lowercase bits so we need only 1 letter( if K is bytes if k is in bits ... seen this way of handling it in many apps)
marian-pritsak
force-pushed
the
patch-4
branch
2 times, most recently
from
7c101b7 to
9e9341d
Compare
Signed-off-by: Marian Pritsak <[email protected]>
marian-pritsak
force-pushed
the
patch-4
branch
2 times, most recently
from
152800a to
c944bda
Compare
|
Same SAI Challenger issues |
|
@mgheorghe or @chrispsommers - would you be able to help with this one (test failing)? If N, no worries. |
|
Lower priority possibly as attribute is not used yet. BUT, it could likely be merged after fixes, @marian-pritsak thinks it can be done now. Update SAI headers, need to keep a stable version of SAI headers. Keep pending for now. |
No description provided.