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

WIP: Add Ethernet intf subscription support #29

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

Conversation

nagarwal03
Copy link
Owner

No description provided.

if (ifName == "*") && (inParams.oper == SUBSCRIBE) {
log.Info("intf_table_xfmr * ifName subscribe with targetUriPath ", targetUriPath)

// need to check if to add subinterface tbl !!!!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this subinterface releated comment, since it's not supported now.

}

result.dbDataMap = RedisDbSubscribeMap{db.ConfigDB: {
"PORT": {ifName: {"autoneg": "auto-negotiate", "adv_speeds": "advertised-speed", "link_training": "standalone-link-training", "unreliable_los": "unreliable-los", "speed": "port-speed", "fec": "port-fec"}}}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check and remove the fields that we don't support already in community-sonic as part of OC-interfaces support our previous PR (sonic-net#125).

For e.g. These fields we didn't support in previous PR.

  • advertised-speed
  • standalone-link-training
  • unreliable-los
  • port-fec

@@ -47,6 +47,8 @@ func init() {
XlateFuncBind("DbToYang_intf_enabled_xfmr", DbToYang_intf_enabled_xfmr)
XlateFuncBind("YangToDb_intf_eth_port_config_xfmr", YangToDb_intf_eth_port_config_xfmr)
XlateFuncBind("DbToYang_intf_eth_port_config_xfmr", DbToYang_intf_eth_port_config_xfmr)
XlateFuncBind("Subscribe_intf_eth_port_config_xfmr", Subscribe_intf_eth_port_config_xfmr)
XlateFuncBind("DbToYangPath_intf_eth_port_config_path_xfmr", DbToYangPath_intf_eth_port_config_path_xfmr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

you will have to add path-config xfmr in annotation file

    deviation /oc-intf:interfaces/oc-intf:interface/oc-eth:ethernet/oc-eth:config {
        deviate add {
            sonic-ext:path-transformer "intf_eth_port_config_path_xfmr";
        }
    }

return tblList, nil
}

intfType, _, ierr := getIntfTypeByName(ifName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need this getIntfTypeByName code here? because subscribe code returns with above if-check. And this check was not there earlier in previous PR for GET/SET support. This question might come from external review. We should try to remove this code and test how things are.

@@ -973,11 +1089,14 @@ var YangToDb_intf_subintfs_xfmr KeyXfmrYangToDb = func(inParams XfmrParams) (str

log.Info("YangToDb_intf_subintfs_xfmr: i32: %s", i32)

if i32 != 0 {
if idx != "0" && idx != "*" && idx != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i32 is getting unused here. We can remove i32 variable fetch code and variable itself, if not needed. Please test, GET/SET works now for subinterfaces index value != 0 now. Earlier with previous PR, it was throwing error "Subinterfaces not supported".


log.Infof("path:%v ifKey:%v, ipKey:%v tbl:[%v]", origTargetUriPath, ifKey, ipKey, tableName)

ipKey = pathInfo.Var("ip")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lines (2221 to 2230) seems repeated again.. Same as lines (2210 to 2219)

@nagarwal03 nagarwal03 marked this pull request as ready for review May 14, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants