Skip to content

[dhcp_relay] Correct to use port alias instead of port name#998

Merged
jleveque merged 1 commit intosonic-net:masterfrom
gord1306:dhcp_relay_get_port_alias
Jul 9, 2019
Merged

[dhcp_relay] Correct to use port alias instead of port name#998
jleveque merged 1 commit intosonic-net:masterfrom
gord1306:dhcp_relay_get_port_alias

Conversation

@gord1306
Copy link
Copy Markdown
Contributor

@gord1306 gord1306 commented Jul 8, 2019

Description of PR

#976 dhcp_relay daemon use port interface's alias to fill option82 circuit ID instead of using port inteface's name.
In current yml, use minigraph_vlans' member as client port alias name, but there are only port interface's name in minigraph_vlans. Therefore add to use minigraph_port_name_to_alias_map to obtain the port interface's alias

Summary:
Fixes # (issue)

Type of change

  • [] Bug fix

Approach

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

  The dhcp_relay daemon use port interface's alias to fill option82 circuit ID instead of using port inteface's name.
In current yml, use minigraph_vlans' member as client port alias name, but there are only port interface's name in minigraph_vlans. Therefore add to use minigraph_port_name_to_alias_map to obtain the port interface's alias
@gord1306 gord1306 mentioned this pull request Jul 9, 2019
@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Jul 9, 2019

Resolves #976.

Thanks, @gord1306 !

@jleveque jleveque merged commit b310657 into sonic-net:master Jul 9, 2019
@gord1306 gord1306 deleted the dhcp_relay_get_port_alias branch July 13, 2019 14:30
yxieca pushed a commit that referenced this pull request Sep 26, 2019
The dhcp_relay daemon use port interface's alias to fill option82 circuit ID instead of using port inteface's name.
In current yml, use minigraph_vlans' member as client port alias name, but there are only port interface's name in minigraph_vlans. Therefore add to use minigraph_port_name_to_alias_map to obtain the port interface's alias
sdszhang added a commit to sdszhang/sonic-mgmt that referenced this pull request Mar 23, 2026
…dia lane count attributes preset in TRANSCEIVER_INFO for add rack test case (sonic-net#998)

### Description of PR

Issue was exposed as part of test_add_rack.py on an internal cisco run.
The optic module type on the setup is `OSFP 8X Pluggable Transceiver`
which supports attributes - apsel (application select), media_lane,
host_lane count.
As part of add rack test case, it is testing adding/removing a T0
neighbor on a existing T1 setup by [1] config reload(with T0, without
T0) and [2] config apply-patch(patch-add, patch remove) CLI and as part
of validation it compares the DB data pre and post config application.
Currently we see a failure on db comparison after patch_rm on
TRANSCEIVER_INFO table in state-db on these attributes-
active_apsel[1,2,3..], media_lane and host_lane count. Going through
xcvrd code;
When admin_status is updated to status ‘up’ through patch add config,
CMIS states transitions through the below states and transceiver info
gets updated to DB at the final state ‘CMIS_STATE_DP_ACTIVATE'
[sonic-platform-daemons/sonic-xcvrd/xcvrd/xcvrd.py at 202405 ·
sonic-net/sonic-platform-daemons](https://github.com/sonic-net/sonic-platform-daemons/blob/202405/sonic-xcvrd/xcvrd/xcvrd.py#L1490)
CMIS_STATE_INSERTED → CMIS_STATE_DP_DEINIT → CMIS_STATE_AP_CONF →
CMIS_STATE_DP_INIT → CMIS_STATE_DP_TXON → CMIS_STATE_DP_ACTIVATE - >
CMIS_STATE_INSERTED
On patch rm- removal of admin status and config interface shutdown ,
CMIS state remains unchanged and it only disables transmission channel
and returns from this point and does NOT parse through the flow which
updates the transceiver info to DB.
[sonic-platform-daemons/sonic-xcvrd/xcvrd/xcvrd.py at 202405 ·
sonic-net/sonic-platform-daemons](https://github.com/sonic-net/sonic-platform-daemons/blob/202405/sonic-xcvrd/xcvrd/xcvrd.py#L1310)
As the interfaces are already configured and these attributes do not
change as part of shutdown of admin status, db data on said attributes
not changing as part of patch rm is in alignment with xcvrd code flow.
This will not be applicable to copper modules as these attributes are
not supported on them. It is possible that this test was written at the
time when these optic modules were not actively supported/used and would
need to be be handled now as part of the test case.

Summary:
Fixes # (issue)

### Type of change

- [ ] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
    - [ ] Skipped for non-supported platforms
- [x] Test case improvement

### Back port request
- [ ] 202012
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [x] 202405
- [x] 202411

### Approach
#### What is the motivation for this PR?

#### How did you do it?

#### How did you verify/test it?
Run test_add_rack.py case with the change
```
============================================================================================================== warnings summary ==============================================================================================================
../../usr/local/lib/python3.8/dist-packages/paramiko/transport.py:236
  /usr/local/lib/python3.8/dist-packages/paramiko/transport.py:236: CryptographyDeprecationWarning: Blowfish has been deprecated
    "class": algorithms.Blowfish,

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
------------------------------------------------------------------------------- generated xml file: /run_logs/configlet/test_add_rack_2025-02-18-21-46-08.xml --------------------------------------------------------------------------------
INFO:root:Can not get Allure report URL. Please check logs
----------------------------------------------------------------------------------------------------------- live log sessionfinish -----------------------------------------------------------------------------------------------------------
22:16:25 __init__.pytest_terminal_summary         L0067 INFO   | Can not get Allure report URL. Please check logs
================================================================================================= 1 passed, 1 warning in 1816.40s (0:30:16) ==================================================================================================
```

#### Any platform specific information?

#### Supported testbed topology if it's a new test case?

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants