feat: Added support for TLV_CODE_VENDOR_NAME by gupurush · Pull Request #542 · sonic-net/sonic-platform-common

gupurush · 2025-02-13T01:05:28Z

Description

• Fix Option 60 encoding.
	Update dhclient.conf.j2 templated to add "\0x00" as first character in the string to indicate that included id is a string
• Add support for Option 61
	To add vendor-class unconditionally in each message, updated the dhclient.conf.j2 to included vendor-name:product-name. The value for these variables is retrieved from eeprom. Product name is already available and added the option to retrieve vendor name thru decode-syseeprom utility.

Required PRs:
sonic-net/sonic-utilities#3765
sonic-net/sonic-buildimage#21722

Logs:

# =============== Managed by SONiC Config Engine DO NOT EDIT! ===============
# generated from /usr/share/sonic/templates/dhclient.conf.j2 using sonic-cfggen
# file: /etc/dhcp/dhclient.conf
#
# Configuration file for /sbin/dhclient, which is included in Debian's
#       dhcp3-client package.
#
# This is a sample configuration file for dhclient. See dhclient.conf's
#       man page for more information about the syntax of this file
#       and a more comprehensive list of the parameters understood by
#       dhclient.
#
# Normally, if the DHCP server provides reasonable information and does
#       not leave anything out (like the domain name, for example), then
#       few changes must be made to this file, if any.
#

option rfc3442-classless-static-routes code 121 = array of unsigned integer 8;
option snmp-community code 224 = text;
option minigraph-url code 225 = text;
option acl-url code 226 = text;
option tftp-server-name code 66 = text;
option bootfile-name code 67 = text;
option user-class code 77 = text;
option provisioning-script-url code 239 = text;
option dhcp6.user-class code 15 = text;
option dhcp6.provisioning-script-url code 239 = text;
option dhcp6.boot-file-url code 59 = text;
option vendor-class code 60 = text;

send host-name = gethostname();
request subnet-mask, broadcast-address, time-offset, routers,
        domain-name, domain-name-servers, domain-search, host-name,
        dhcp6.name-servers, dhcp6.domain-search, interface-mtu, dhcp6.fqdn,
        rfc3442-classless-static-routes, ntp-servers, log-servers,snmp-community, minigraph-url, acl-url;

send vendor-class "Cisco:{MODEL-XXXX}"; 


18:31:56.604658 IP (tos 0x0, ttl 64, id 58732, offset 0, flags [DF], proto UDP (17), length 338)
    10.29.XXX.XXX.bootpc > hostname.example.com.bootps: [udp sum ok] BOOTP/DHCP, Request from 34:73:YY:YY:YY:YY (oui Unknown), length 310, xid 0xff8d9300, Flags [none] (0x0000)
          Client-IP 10.29.XXX.XXX
          Client-Ethernet-Address 34:73:YY:YY:YY:YY (oui Unknown)
          Vendor-rfc1048 Extensions
            Magic Cookie 0x63825363
            DHCP-Message Option 53, length 1: Release
            Server-ID Option 54, length 4: hostname.example.com
            Hostname Option 12, length 5: "sonic"
            Vendor-Class Option 60, length 18: "Cisco:{MODEL-XXXX}"
            Client-ID Option 61, length 31: "SONiC##MODEL-XXXX##XYZ" 
            
            
            
root@sonic:/home/cisco# ifconfig eth0
eth0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet6 fe80::3673:2dff:fe30:70d8  prefixlen 64  scopeid 0x20<link>
        ether 34:73:YY:YY:YY:YY  txqueuelen 1000  (Ethernet)
        RX packets 9939  bytes 2618950 (2.4 MiB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 1219  bytes 203002 (198.2 KiB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
        device memory 0x95000000-9501ffff 
        ```

mssonicbld · 2025-02-13T01:05:31Z

/azp run

linux-foundation-easycla · 2025-02-13T01:05:32Z

The committers listed above are authorized under a signed CLA.

✅ login: gupurush / name: Nanma Purushotam (318b147, 007e93b)

azure-pipelines · 2025-02-13T01:05:39Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-02-13T01:06:35Z

/azp run

azure-pipelines · 2025-02-13T01:06:43Z

Azure Pipelines successfully started running 1 pipeline(s).

FengPan-Frank · 2025-02-13T13:08:10Z

sonic_platform_base/sonic_eeprom/eeprom_tlvinfo.py

        return t[2].decode("ascii")
+
+
+    def vendorstr(self, e):


pls add test in https://github.com/sonic-net/sonic-platform-common/blob/cb5564c20ac74694f2391759f9235eee428a97d0/tests/eeprom_base_test.py

Tests added as requested.

mssonicbld · 2025-03-05T00:38:18Z

/azp run

azure-pipelines · 2025-03-05T00:38:25Z

Azure Pipelines successfully started running 1 pipeline(s).

gupurush · 2025-03-14T17:00:50Z

Hey @FengPan-Frank , please help review

ashutosh-agrawal · 2025-03-14T18:34:41Z

@yaqiangz Appreciate your help in reviewing this PR.

Copilot

Pull request overview

Adds decoding support for the EEPROM TLV “Vendor Name” field (TLV code 0x2D) in TlvInfoDecoder, along with a new v2 EEPROM hex fixture and an accompanying unit test to validate Vendor Name decoding.

Changes:

Added vendorstr() API to TlvInfoDecoder to decode TLV code 0x2D as an ASCII string.
Added a new EEPROM hex fixture (syseeprom_v2.hex) that includes Vendor Name TLV data.
Extended unit tests to generate a v2 mock EEPROM blob and assert Vendor Name decoding.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
`sonic_platform_base/sonic_eeprom/eeprom_tlvinfo.py`	Adds `vendorstr()` accessor for Vendor Name TLV decoding.
`tests/eeprom_base_test.py`	Creates v2 mock EEPROM file and adds assertions for Vendor Name.
`tests/syseeprom_v2.hex`	New test fixture EEPROM hex containing Vendor Name TLV.

Copilot · 2026-03-10T14:11:33Z

sonic_platform_base/sonic_eeprom/eeprom_tlvinfo.py

+        '''
+        (is_valid, t) = self.get_tlv_field(e, self._TLV_CODE_VENDOR_NAME)
+        if not is_valid:
+            return super(TlvInfoDecoder, self).vendorstr(e)


vendorstr() falls back to super(...).vendorstr(e) when the TLV isn't present, but the base EepromDecoder/parent classes don't define vendorstr. This will raise AttributeError for EEPROMs that don't include _TLV_CODE_VENDOR_NAME. Consider returning a safe default (e.g., empty string) when the TLV is missing, or add a vendorstr() implementation to the parent class that returns a default value.

Suggested change

return super(TlvInfoDecoder, self).vendorstr(e)

# Parent class does not define vendorstr(); return a safe default

return ""

Copilot · 2026-03-10T14:11:34Z

tests/eeprom_base_test.py

@@ -38,12 +40,15 @@ def setup_class(cls):
        if not os.path.exists(os.path.dirname(EEPROM_HEX_FILE_FULL_PATH)):
            assert False, "File {} is not exist".format(EEPROM_HEX_FILE_FULL_PATH)
        subprocess.check_call(['/usr/bin/xxd', '-r', '-p', EEPROM_HEX_FILE_FULL_PATH, EEPROM_SYMLINK_FULL_PATH])


The new v2 EEPROM fixture is consumed via xxd without verifying the file exists first. Adding an explicit os.path.exists(EEPROM_HEX_FILE_V2) assertion (similar to the existing v1 check) would produce a clearer test failure than xxd's CalledProcessError if the fixture is missing or misnamed.

Suggested change

subprocess.check_call(['/usr/bin/xxd', '-r', '-p', EEPROM_HEX_FILE_FULL_PATH, EEPROM_SYMLINK_FULL_PATH])

subprocess.check_call(['/usr/bin/xxd', '-r', '-p', EEPROM_HEX_FILE_FULL_PATH, EEPROM_SYMLINK_FULL_PATH])

if not os.path.exists(EEPROM_HEX_FILE_V2):

assert False, "File {} is not exist".format(EEPROM_HEX_FILE_V2)

Copilot · 2026-03-10T14:11:34Z

tests/eeprom_base_test.py

@@ -57,6 +62,19 @@ def test_eeprom_tlvinfo_read_api(self):
        assert(eeprom_class.serial_number_str(eeprom).rstrip('\0') == 'MT1623X09522')
        assert(eeprom_class.part_number_str(eeprom).rstrip('\0') == 'MSN2700-CS2FO')


There is coverage for the happy path where Vendor Name TLV exists, but no test exercises vendorstr() behavior when the TLV is absent (e.g., using the existing v1 syseeprom.hex). After fixing the fallback behavior, consider adding an assertion in test_eeprom_tlvinfo_read_api for the expected default (empty string/None/etc.) to prevent regressions.

Copilot · 2026-03-10T14:11:34Z

tests/eeprom_base_test.py

+    def test_eeprom_tlvinfo_read_api_v2(self):
+        # Test using the updated api (addition of Vendor Name) to fetch Base MAC, Model, Vendor Name,
+        # Serial Number and Part Number.
+        eeprom_class = eeprom_tlvinfo.TlvInfoDecoder(EEPROM_SYMLINK_V2, 0, '', True)
+        eeprom = eeprom_class.read_eeprom()
+        eeprom_class.decode_eeprom(eeprom)
+        assert(eeprom_class.base_mac_addr(eeprom).rstrip('\0') == '34:73:2D:30:70:D8')
+        assert(eeprom_class.switchaddrrange(eeprom).rstrip('\0') == '516')
+        assert(eeprom_class.modelstr(eeprom).rstrip('\0') == '8102-64H-O')
+        assert(eeprom_class.vendorstr(eeprom).rstrip('\0') == 'Cisco')
+        assert(eeprom_class.serial_number_str(eeprom).rstrip('\0') == 'FLM251907U7')
+        assert(eeprom_class.part_number_str(eeprom).rstrip('\0') == 'ECI123')


The PR description references dhclient.conf.j2 changes for DHCP option 60/61, but this repository/PR only appears to update the EEPROM TLV decoder and tests (no dhclient.conf.j2 present/modified here). Consider updating the PR description to match the actual scope of this repo change, or link to the other PRs where the DHCP config changes live.

feat: Added support for TLV_CODE_VENDOR_NAME

007e93b

gupurush force-pushed the feat/dhcp branch from d0947bb to 007e93b Compare February 13, 2025 01:06

gupurush mentioned this pull request Feb 13, 2025

Fixed dhcp Option 60 encoding sonic-net/sonic-buildimage#21722

Open

FengPan-Frank reviewed Feb 13, 2025

View reviewed changes

add vendor tests with new updated syseeprom contents

318b147

gupurush requested a review from FengPan-Frank March 5, 2025 00:41

gupurush mentioned this pull request Mar 14, 2025

Added support for TLV_CODE_VENDOR_NAME sonic-net/sonic-utilities#3765

Open

FengPan-Frank approved these changes May 13, 2025

View reviewed changes

lihuay requested a review from Copilot March 10, 2026 14:08

Copilot started reviewing on behalf of lihuay March 10, 2026 14:08 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

	return super(TlvInfoDecoder, self).vendorstr(e)
	# Parent class does not define vendorstr(); return a safe default
	return ""

		@@ -57,6 +62,19 @@ def test_eeprom_tlvinfo_read_api(self):
		assert(eeprom_class.serial_number_str(eeprom).rstrip('\0') == 'MT1623X09522')
		assert(eeprom_class.part_number_str(eeprom).rstrip('\0') == 'MSN2700-CS2FO')

Conversation

gupurush commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

mssonicbld commented Feb 13, 2025

Uh oh!

linux-foundation-easycla bot commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines bot commented Feb 13, 2025

Uh oh!

mssonicbld commented Feb 13, 2025

Uh oh!

azure-pipelines bot commented Feb 13, 2025

Uh oh!

FengPan-Frank Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

gupurush Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Mar 5, 2025

Uh oh!

azure-pipelines bot commented Mar 5, 2025

Uh oh!

gupurush commented Mar 14, 2025

Uh oh!

ashutosh-agrawal commented Mar 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gupurush commented Feb 13, 2025 •

edited

Loading

linux-foundation-easycla bot commented Feb 13, 2025 •

edited

Loading