Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION by longhuan-cisco · Pull Request #3764 · sonic-net/sonic-swss

longhuan-cisco · 2025-07-14T23:03:59Z

sonic-platform-daemon side change: sonic-net/sonic-platform-daemons#643

What I did
Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION (i.e. custom serdes attributes represented in JSON based string format)

Use boost::variant for the serdes_attr map value to support both std::vector<uint32_t> and std::string in a type-safe way, with easy extensibility to add more types later.

Why I did it

How I verified it
Tested end-to-end xcvrd->OA->SAI/SDK

Covered below 3 scenarios:

attr_list only contains traditional (non-JSON based) serdes attributes
attr_list only contains custom (JSON based) serdes attributes
attr_list contains a mixed of both

Details if related

mssonicbld · 2025-07-14T23:04:07Z

/azp run

azure-pipelines · 2025-07-14T23:04:16Z

Azure Pipelines successfully started running 1 pipeline(s).

longhuan-cisco · 2025-07-14T23:18:01Z

@prgeor @mihirpat1 could you please review, thanks

orchagent/portsorch.cpp

mssonicbld · 2025-08-11T22:48:29Z

/azp run

azure-pipelines · 2025-08-11T22:48:39Z

Azure Pipelines successfully started running 1 pipeline(s).

orchagent/port/porthlpr.cpp

… for serdes attrs

mssonicbld · 2025-09-10T21:29:29Z

/azp run

azure-pipelines · 2025-09-10T21:29:39Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-09-11T01:26:05Z

/azp run

azure-pipelines · 2025-09-11T01:26:14Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-09-11T18:55:02Z

/azp run

azure-pipelines · 2025-09-11T18:55:13Z

Azure Pipelines successfully started running 1 pipeline(s).

orchagent/port/porthlpr.cpp

prgeor · 2025-10-27T20:26:14Z

@prsunny can you merge?

longhuan-cisco · 2025-11-08T02:01:51Z

@prsunny can you merge?

@prsunny kindly reminder, thanks

prsunny · 2025-11-10T23:30:33Z

@longhuan-cisco , i see new boost library added, can you provide description of why its added?

azure-pipelines · 2025-11-15T01:33:18Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2025-11-15T01:33:26Z

/azp run

azure-pipelines · 2025-11-15T01:33:34Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-19T22:14:03Z

/azp run

azure-pipelines · 2025-11-19T22:14:13Z

Azure Pipelines successfully started running 1 pipeline(s).

longhuan-cisco · 2025-11-21T01:10:28Z

Looks like, Azure.sonic-swss failure was due to base line issue, which was hit on other PRs also

mssonicbld · 2025-11-21T02:49:51Z

/azp run

azure-pipelines · 2025-11-21T02:50:01Z

Azure Pipelines successfully started running 1 pipeline(s).

longhuan-cisco · 2025-11-21T22:30:33Z

I saw Semgrep pre-commit run got stuck in "Waiting for status to be reported" state for quite some time, not sure if it's due to some issue in Azure Pipelines

cc @prsunny

mssonicbld · 2025-11-24T23:34:27Z

/azp run

azure-pipelines · 2025-11-24T23:34:38Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-26T01:12:36Z

/azp run

azure-pipelines · 2025-11-26T01:12:45Z

Azure Pipelines successfully started running 1 pipeline(s).

longhuan-cisco · 2025-11-26T22:29:10Z

@prsunny kindly reminder for approval/merge. all pre-commit checks passed.

balanokia · 2025-12-15T11:44:44Z

@prsunny @prgeor @lguohan : Pls shared review comments. We made this PR to introduce boost:variant type but it changes keywords in a non-functional way.
Hi @longhuan-cisco
Tx FIR Pre-emphasis is the key word for Tx FIR Equalization with certain number of taps and Pre-emphasis keyword is used in IEEE or all OEM datasheets in a functional meaning. Lets not change the keyword from pre-emphasis to generic keyword "serdes_attrs" and "serdes_attr" does not signify Tx FIR (tap) value set. Lets keep the code understanding in a functional meaning. Kindly share your views.
Thanks
Bala

longhuan-cisco · 2025-12-15T18:40:51Z

Hi @longhuan-cisco
Tx FIR Pre-emphasis is the key word for Tx FIR Equalization with certain number of taps and Pre-emphasis keyword is used in IEEE or all OEM datasheets in a functional meaning. Lets not change the keyword from pre-emphasis to generic keyword "serdes_attrs" and "serdes_attr" does not signify Tx FIR (tap) value set. Lets keep the code understanding in a functional meaning. Kindly share your views.
Thanks
Bala

Hi @balanokia
In the existing code before this PR, this variable had been used to cover many different serdes attributes for quite long time, including SAI_PORT_SERDES_ATTR_PREEMPHASIS, SAI_PORT_SERDES_ATTR_IDRIVER, SAI_PORT_SERDES_ATTR_IPREDRIVER and etc.
It's generic, not just for SAI_PORT_SERDES_ATTR_PREEMPHASIS

Please refer to below old code snapshot (before this PR merged):

sonic-swss/orchagent/portsorch.cpp

Lines 435 to 524 in 5671e08

    
           static void getPortSerdesAttr(PortSerdesAttrMap_t &map, const PortConfig &port) 
        
           { 
        
               if (port.serdes.preemphasis.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_PREEMPHASIS] = port.serdes.preemphasis.value; 
        
               } 
        
               if (port.serdes.idriver.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_IDRIVER] = port.serdes.idriver.value; 
        
               } 
        
               if (port.serdes.ipredriver.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_IPREDRIVER] = port.serdes.ipredriver.value; 
        
               } 
        
               if (port.serdes.pre1.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE1] = port.serdes.pre1.value; 
        
               } 
        
               if (port.serdes.pre2.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE2] = port.serdes.pre2.value; 
        
               } 
        
               if (port.serdes.pre3.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE3] = port.serdes.pre3.value; 
        
               } 
        
               if (port.serdes.main.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_MAIN] = port.serdes.main.value; 
        
               } 
        
               if (port.serdes.post1.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_POST1] = port.serdes.post1.value; 
        
               } 
        
               if (port.serdes.post2.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_POST2] = port.serdes.post2.value; 
        
               } 
        
               if (port.serdes.post3.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_POST3] = port.serdes.post3.value; 
        
               } 
        
               if (port.serdes.attn.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_FIR_ATTN] = port.serdes.attn.value; 
        
               } 
        
               if (port.serdes.ob_m2lp.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_PAM4_RATIO] = port.serdes.ob_m2lp.value; 
        
               } 
        
               if (port.serdes.ob_alev_out.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_OUT_COMMON_MODE] = port.serdes.ob_alev_out.value; 
        
               } 
        
               if (port.serdes.obplev.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_PMOS_COMMON_MODE] = port.serdes.obplev.value; 
        
               } 
        
               if (port.serdes.obnlev.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_NMOS_COMMON_MODE] = port.serdes.obnlev.value; 
        
               } 
        
               if (port.serdes.regn_bfm1p.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_PMOS_VLTG_REG] = port.serdes.regn_bfm1p.value; 
        
               } 
        
               if (port.serdes.regn_bfm1n.is_set) 
        
               { 
        
                   map[SAI_PORT_SERDES_ATTR_TX_NMOS_VLTG_REG] = port.serdes.regn_bfm1n.value; 
        
               } 
        
           }

sonic-swss/orchagent/port/portschema.h

Lines 75 to 91 in 5671e08

    
           #define PORT_PREEMPHASIS           "preemphasis" 
        
           #define PORT_IDRIVER               "idriver" 
        
           #define PORT_IPREDRIVER            "ipredriver" 
        
           #define PORT_PRE1                  "pre1" 
        
           #define PORT_PRE2                  "pre2" 
        
           #define PORT_PRE3                  "pre3" 
        
           #define PORT_MAIN                  "main" 
        
           #define PORT_POST1                 "post1" 
        
           #define PORT_POST2                 "post2" 
        
           #define PORT_POST3                 "post3" 
        
           #define PORT_ATTN                  "attn" 
        
           #define PORT_OB_M2LP               "ob_m2lp" 
        
           #define PORT_OB_ALEV_OUT           "ob_alev_out" 
        
           #define PORT_OBPLEV                "obplev" 
        
           #define PORT_OBNLEV                "obnlev" 
        
           #define PORT_REGN_BFM1P            "regn_bfm1p" 
        
           #define PORT_REGN_BFM1N            "regn_bfm1n"

sonic-swss/orchagent/portsorch.cpp

Lines 4492 to 4493 in 5671e08

    
           PortSerdesAttrMap_t serdes_attr; 
        
           getPortSerdesAttr(serdes_attr, pCfg);

sonic-swss/orchagent/portsorch.cpp

Line 5124 in 5671e08

p.m_preemphasis = serdes_attr;

sonic-platform-daemon side change: sonic-net/sonic-platform-daemons#643 What I did Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION (i.e. custom serdes attributes represented in JSON based string format) Use boost::variant for the serdes_attr map value to support both std::vector<uint32_t> and std::string in a type-safe way, with easy extensibility to add more types later. Signed-off-by: Kalash Nainwal <kalash@nexthop.ai>

balanokia · 2025-12-23T19:33:53Z

Hi @longhuan-cisco
Tx FIR Pre-emphasis is the key word for Tx FIR Equalization with certain number of taps and Pre-emphasis keyword is used in IEEE or all OEM datasheets in a functional meaning. Lets not change the keyword from pre-emphasis to generic keyword "serdes_attrs" and "serdes_attr" does not signify Tx FIR (tap) value set. Lets keep the code understanding in a functional meaning. Kindly share your views.
Thanks
Bala

Hi @balanokia In the existing code before this PR, this variable had been used to cover many different serdes attributes for quite long time, including SAI_PORT_SERDES_ATTR_PREEMPHASIS, SAI_PORT_SERDES_ATTR_IDRIVER, SAI_PORT_SERDES_ATTR_IPREDRIVER and etc. It's generic, not just for SAI_PORT_SERDES_ATTR_PREEMPHASIS

Please refer to below old code snapshot (before this PR merged):

sonic-swss/orchagent/portsorch.cpp

Lines 435 to 524 in 5671e08

static void getPortSerdesAttr(PortSerdesAttrMap_t &map, const PortConfig &port)

{

if (port.serdes.preemphasis.is_set)

{

map[SAI_PORT_SERDES_ATTR_PREEMPHASIS] = port.serdes.preemphasis.value;

}

if (port.serdes.idriver.is_set)

{

map[SAI_PORT_SERDES_ATTR_IDRIVER] = port.serdes.idriver.value;

}

if (port.serdes.ipredriver.is_set)

{

map[SAI_PORT_SERDES_ATTR_IPREDRIVER] = port.serdes.ipredriver.value;

}

if (port.serdes.pre1.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE1] = port.serdes.pre1.value;

}

if (port.serdes.pre2.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE2] = port.serdes.pre2.value;

}

if (port.serdes.pre3.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_PRE3] = port.serdes.pre3.value;

}

if (port.serdes.main.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_MAIN] = port.serdes.main.value;

}

if (port.serdes.post1.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_POST1] = port.serdes.post1.value;

}

if (port.serdes.post2.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_POST2] = port.serdes.post2.value;

}

if (port.serdes.post3.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_POST3] = port.serdes.post3.value;

}

if (port.serdes.attn.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_FIR_ATTN] = port.serdes.attn.value;

}

if (port.serdes.ob_m2lp.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_PAM4_RATIO] = port.serdes.ob_m2lp.value;

}

if (port.serdes.ob_alev_out.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_OUT_COMMON_MODE] = port.serdes.ob_alev_out.value;

}

if (port.serdes.obplev.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_PMOS_COMMON_MODE] = port.serdes.obplev.value;

}

if (port.serdes.obnlev.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_NMOS_COMMON_MODE] = port.serdes.obnlev.value;

}

if (port.serdes.regn_bfm1p.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_PMOS_VLTG_REG] = port.serdes.regn_bfm1p.value;

}

if (port.serdes.regn_bfm1n.is_set)

{

map[SAI_PORT_SERDES_ATTR_TX_NMOS_VLTG_REG] = port.serdes.regn_bfm1n.value;

}

}

sonic-swss/orchagent/port/portschema.h

Lines 75 to 91 in 5671e08

#define PORT_PREEMPHASIS "preemphasis"

#define PORT_IDRIVER "idriver"

#define PORT_IPREDRIVER "ipredriver"

#define PORT_PRE1 "pre1"

#define PORT_PRE2 "pre2"

#define PORT_PRE3 "pre3"

#define PORT_MAIN "main"

#define PORT_POST1 "post1"

#define PORT_POST2 "post2"

#define PORT_POST3 "post3"

#define PORT_ATTN "attn"

#define PORT_OB_M2LP "ob_m2lp"

#define PORT_OB_ALEV_OUT "ob_alev_out"

#define PORT_OBPLEV "obplev"

#define PORT_OBNLEV "obnlev"

#define PORT_REGN_BFM1P "regn_bfm1p"

#define PORT_REGN_BFM1N "regn_bfm1n"

sonic-swss/orchagent/portsorch.cpp

Lines 4492 to 4493 in 5671e08

PortSerdesAttrMap_t serdes_attr;

getPortSerdesAttr(serdes_attr, pCfg);

sonic-swss/orchagent/portsorch.cpp

Line 5124 in 5671e08

p.m_preemphasis = serdes_attr;

Hi @longhuan-cisco
Thanks for clarifying. Its good to have abstraction of serdes attributes under one bucket. When we invoke to apply a specific type of serdes attributes, we know what significant parameters that we update and hence it would be better to keep the variable names or atleast keep the log messages as-is in the past like "SWSS_LOG_NOTICE("Save port %s preemphasis for LT", p.m_alias.c_str())". It would help us to debug port bringup sequences (LT, FEC, Tx FIR, etc) especially external PHY stages.

Thanks
Bala

longhuan-cisco · 2026-01-05T23:43:40Z

Hi @longhuan-cisco Thanks for clarifying. Its good to have abstraction of serdes attributes under one bucket. When we invoke to apply a specific type of serdes attributes, we know what significant parameters that we update and hence it would be better to keep the variable names or atleast keep the log messages as-is in the past like "SWSS_LOG_NOTICE("Save port %s preemphasis for LT", p.m_alias.c_str())". It would help us to debug port bringup sequences (LT, FEC, Tx FIR, etc) especially external PHY stages.

Thanks
Bala

Hi @balanokia, thanks for the context on PHY terminology.

In general PHY literature, “pre‑emphasis” is indeed often used to refer to TX equalization and is sometimes implemented/expressed via TX‑FIR/FFE taps.

However, in SONiC orchagent this saved/restored bucket has historically been a generic SERDES attribute bucket, not strictly “pre‑emphasis only”. It can include multiple SAI_PORT_SERDES_ATTR_* knobs depending on platform media_settings.json/config (e.g., PREEMPHASIS, IDRIVER/IPREDRIVER, TX_FIR_* taps, PAM4 ratio/common-mode controls, precoding, etc.). SAI itself defines these as distinct SERDES attributes, where PREEMPHASIS is just one item in the list.

Because the bucket is not guaranteed to contain PREEMPHASIS on every platform/vendor/module, keeping “preemphasis” as the generic bucket/variable/log keyword is semantically misleading and can oversimplify what is actually being saved/restored. Renaming it to serdes_attrs improves correctness without changing behavior or reducing log informativeness—the previous logs did not actually confirm whether PREEMPHASIS was present either.

For debuggability, a more robust community-wide improvement would be to log which SERDES attributes are being set/saved/restored (e.g., list attr IDs/names at NOTICE, and optionally values at DEBUG), so anyone can grep for a stable tag and still see the exact contents. This makes the behavior transparent across all vendors/platforms/config combinations.

Thanks,
Longyin

sonic-platform-daemon side change: sonic-net/sonic-platform-daemons#643 What I did Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION (i.e. custom serdes attributes represented in JSON based string format) Use boost::variant for the serdes_attr map value to support both std::vector<uint32_t> and std::string in a type-safe way, with easy extensibility to add more types later.

sonic-platform-daemon side change: sonic-net/sonic-platform-daemons#643 What I did Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION (i.e. custom serdes attributes represented in JSON based string format) Use boost::variant for the serdes_attr map value to support both std::vector<uint32_t> and std::string in a type-safe way, with easy extensibility to add more types later. Signed-off-by: Baorong Liu <96146196+baorliu@users.noreply.github.com>

longhuan-cisco · 2026-03-26T23:02:35Z

@prgeor Could we cherry-pick this to 202511

balanokia · 2026-03-30T06:50:53Z

@longhuan-cisco @prgeor Pls suggest if we plan to take this PR to 202511 branch. I already see @longhuan-cisco already raised it for merge to 202511.

Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION

fe44806

longhuan-cisco requested a review from prsunny as a code owner July 14, 2025 23:04

longhuan-cisco mentioned this pull request Jul 14, 2025

Add support for custom serdes attributes sonic-net/sonic-platform-daemons#643

Open

prgeor reviewed Jul 22, 2025

View reviewed changes

orchagent/portsorch.cpp Show resolved Hide resolved

prgeor reviewed Jul 22, 2025

View reviewed changes

orchagent/portsorch.cpp Show resolved Hide resolved

longhuan-cisco requested a review from prgeor July 27, 2025 07:19

Merge branch 'master' into support_custom_serdes

124f027

prgeor reviewed Sep 5, 2025

View reviewed changes

orchagent/port/porthlpr.cpp Outdated Show resolved Hide resolved

Use boost::variant to support both vector<uint32_t> and string values…

181eacb

… for serdes attrs

Fix testcase

9c45758

Merge branch 'master' into support_custom_serdes

715e9d3

longhuan-cisco requested a review from prgeor September 18, 2025 16:40

prgeor reviewed Oct 24, 2025

View reviewed changes

orchagent/port/porthlpr.cpp Show resolved Hide resolved

longhuan-cisco requested a review from prgeor October 27, 2025 07:32

prgeor approved these changes Oct 27, 2025

View reviewed changes

Merge branch 'master' into support_custom_serdes

d4d55b0

Merge branch 'master' into support_custom_serdes

21dc02d

Merge branch 'master' into support_custom_serdes

6f79e0e

Merge branch 'master' into support_custom_serdes

b647405

Merge branch 'master' into support_custom_serdes

24d0447

prsunny merged commit a2decc5 into sonic-net:master Nov 26, 2025
15 checks passed

longhuan-cisco mentioned this pull request Jan 7, 2026

Update Media-based-Port-settings.md to support custom SerDes attributes sonic-net/SONiC#2173

Merged

Conversation

longhuan-cisco commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Jul 14, 2025

Uh oh!

azure-pipelines bot commented Jul 14, 2025

Uh oh!

longhuan-cisco commented Jul 14, 2025

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Aug 11, 2025

Uh oh!

azure-pipelines bot commented Aug 11, 2025

Uh oh!

Uh oh!

mssonicbld commented Sep 10, 2025

Uh oh!

azure-pipelines bot commented Sep 10, 2025

Uh oh!

mssonicbld commented Sep 11, 2025

Uh oh!

azure-pipelines bot commented Sep 11, 2025

Uh oh!

mssonicbld commented Sep 11, 2025

Uh oh!

azure-pipelines bot commented Sep 11, 2025

Uh oh!

Uh oh!

prgeor commented Oct 27, 2025

Uh oh!

longhuan-cisco commented Nov 8, 2025

Uh oh!

prsunny commented Nov 10, 2025

Uh oh!

azure-pipelines bot commented Nov 15, 2025

Uh oh!

prgeor commented Nov 15, 2025

Uh oh!

azure-pipelines bot commented Nov 15, 2025

Uh oh!

mssonicbld commented Nov 19, 2025

Uh oh!

azure-pipelines bot commented Nov 19, 2025

Uh oh!

longhuan-cisco commented Nov 21, 2025

Uh oh!

mssonicbld commented Nov 21, 2025

Uh oh!

azure-pipelines bot commented Nov 21, 2025

Uh oh!

longhuan-cisco commented Nov 21, 2025

Uh oh!

mssonicbld commented Nov 24, 2025

Uh oh!

azure-pipelines bot commented Nov 24, 2025

Uh oh!

mssonicbld commented Nov 26, 2025

Uh oh!

azure-pipelines bot commented Nov 26, 2025

Uh oh!

longhuan-cisco commented Nov 26, 2025

Uh oh!

Uh oh!

balanokia commented Dec 15, 2025

Uh oh!

longhuan-cisco commented Dec 15, 2025

Uh oh!

balanokia commented Dec 23, 2025

Uh oh!

longhuan-cisco commented Jan 5, 2026

Uh oh!

longhuan-cisco commented Mar 26, 2026

Uh oh!

balanokia commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

longhuan-cisco commented Jul 14, 2025 •

edited

Loading