[Mellanox] Use MAC from EEPROM for PortChannels and VLAN Interfaces by andriymoroz-mlnx · Pull Request #1793 · sonic-net/sonic-buildimage

andriymoroz-mlnx · 2018-06-14T15:25:28Z

Signed-off-by: Andriy Moroz [email protected]

- What I did
Updated teams start script to pass MAC from EEPROM to the portchannels config template

- How I did it
updated start.sh which runs in teamd container

- How to verify it
Start SONiC and make sure MACs on PortChannel (and members) or Vlan interface are similar to one returned by command
od -vt x1 -An /sys/bus/i2c/devices/8-0051/eeprom | xargs printf "0x%s " | xargs printf "%02x:" | awk 'BEGIN { FS=":"; i=8+1+2+1} {while(i<NF) {type=$i; len=("0x"$(i+1));if(type!="24") {i=i+2+len} else {print substr($0, (i+1)3+1, len3-1); break}}}'

some bits in last byte may differ

Signed-off-by: Andriy Moroz <[email protected]>

andriymoroz-mlnx · 2018-06-14T17:02:48Z

need to do some tests to implement the same for VLAN interfaces...

pavel-shirshov · 2018-06-14T19:02:51Z

dockers/docker-teamd/start.sh

+if [ "$SONIC_ASIC_TYPE" == "mellanox" ]; then
+    MAC_ADDRESS=$(od -vt x1 -An /sys/bus/i2c/devices/8-0051/eeprom | xargs printf "0x%s " | xargs printf "%02x:" | awk 'BEGIN { FS=":"; i=8+1+2+1} {while(i<NF) {type=$i; len=("0x"$(i+1));if(type!="24") {i=i+2+len} else {print substr($0, (i+1)*3+1, len*3-1); break}}}')
+else
+    MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')


cat /sys/class/net/eth0/address

lguohan

use decode-syseeprom -m

lguohan · 2018-06-19T20:14:25Z

dockers/docker-teamd/start.sh

-MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')
+
+if [ "$SONIC_ASIC_TYPE" == "mellanox" ]; then
+    MAC_ADDRESS=$(od -vt x1 -An /sys/bus/i2c/devices/8-0051/eeprom | xargs printf "0x%s " | xargs printf "%02x:" | awk 'BEGIN { FS=":"; i=8+1+2+1} {while(i<NF) {type=$i; len=("0x"$(i+1));if(type!="24") {i=i+2+len} else {print substr($0, (i+1)*3+1, len*3-1); break}}}')


sudo decode-syseeprom -m to figure out the base mac?

it is not available inside docker container, checking why...
UPD:
they have their own /usr/bin
we share only:
-v /etc/sonic:/etc/sonic:ro
-v /var/run/redis:/var/run/redis:rw
-v /usr/share/sonic/device/$PLATFORM:/usr/share/sonic/platform:ro
-v /usr/share/sonic/device/$PLATFORM/$HWSKU:/usr/share/sonic/hwsku:ro \

Will affect MAC for VLAN interfaces Signed-off-by: Andriy Moroz <[email protected]>

qiluo-msft · 2018-06-21T21:54:32Z

src/sonic-config-engine/sonic_platform.py

+
+    if (version_info['asic_type'] == 'mellanox'):
+        eeprom_file = "/sys/bus/i2c/devices/8-0051/eeprom"
+        if (os.access(eeprom_file, os.R_OK)):


if (os.access(eeprom_file, os.R_OK)): [](start = 8, length = 37)

else, it is an error. #Closed

Signed-off-by: Andriy Moroz <[email protected]>

qiluo-msft · 2018-06-27T16:24:57Z

src/sonic-config-engine/sonic_platform.py

+        get_mac_cmd = "ip link show eth0 | grep ether | awk '{print $2}'"
+
+    proc = subprocess.Popen(get_mac_cmd, shell=True, stdout=subprocess.PIPE)
    (mac, err) = proc.communicate()


err [](start = 10, length = 3)

Handle error before using mac. #Closed

qiluo-msft · 2018-06-27T16:36:27Z

files/build_templates/updategraph.service.j2

 Type=oneshot
+{% if sonic_asic_platform == 'mellanox' -%}
+EnvironmentFile=/host/machine.conf
+ExecStartPre=/bin/bash -c "/usr/share/sonic/device/$onie_platform/hw-management start"


hw-management [](start = 66, length = 13)

Is updategraph a good location for hw-management? It seems has nothing todo with graph. @taoyl-ms #Closed

Agree with Qi. I don't see the reason to put hw-management here, and I don't see the need to change this into a j2 file either. Platform information can also be tested during runtime.

we need drivers to access eeprom
j2 to avoid this code for others

Why not create another service for this?
And, you can create a shell script to enable hw-management in which you can test platform information run time. It is much easier to follow than a build-time j2 rendering.

in current case, when will hw-management be started? Maybe hw-management itself should be a service and make it start early.

qiluo-msft

As comments

Signed-off-by: Andriy Moroz <[email protected]>

pavel-shirshov

As comments

pavel-shirshov · 2018-07-12T21:35:27Z

dockers/docker-teamd/start.sh

+if [ "$SONIC_ASIC_TYPE" == "mellanox" ]; then
+    MAC_ADDRESS=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.mac)
+else
+    MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')


MAC_ADDRESS=$(</sys/class/net/eth0/address)

the case under "else" getting MAC address from the "ip link show eth0" is the old code and used for other platforms
I can change it to what you suggested, but I am not able to test it on non-Mellanox boxes...

pavel-shirshov · 2018-07-12T21:36:58Z

files/build_templates/sonic_debian_extension.j2


 # Copy updategraph script and service file
-sudo cp $IMAGE_CONFIGS/updategraph/updategraph.service  $FILESYSTEM_ROOT/etc/systemd/system/
+j2 files/build_templates/updategraph.service.j2 | sudo tee $FILESYSTEM_ROOT/etc/systemd/system/updategraph.service


why do we need to copy everything to the screen?

to get it in build log, I think
seen it in some other place
suggest to remove tee?

pavel-shirshov · 2018-07-12T21:37:50Z

src/sonic-config-engine/sonic_platform.py

+    if (version_info['asic_type'] == 'mellanox'):
+        get_mac_cmd = "sudo decode-syseeprom -m"
+    else:
+        get_mac_cmd = "ip link show eth0 | grep ether | awk '{print $2}'"


read a file "/sys/class/net/eth0/address"?

like above - not sure about other platforms

pavel-shirshov · 2018-07-12T21:38:23Z

dockers/docker-teamd/start.sh

-MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')
+
+if [ "$SONIC_ASIC_TYPE" == "mellanox" ]; then
+    MAC_ADDRESS=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.mac)


can we do sudo decode-syseeprom -m here?

unfortunately no, it is not available in the containers...

Signed-off-by: Andriy Moroz <[email protected]>

pavel-shirshov

Discussed offline. Let's keep the original PR.

qiluo-msft · 2018-07-18T16:47:41Z

git clone https://github.com/Mellanox/hw-mgmt/ -b $(MLNX_HW_MANAGEMENT_VERSION) hw-management

This is a suggestion. Since the code is owned by Mellanox, why not commit the patch there?

Refers to: platform/mellanox/hw-management/Makefile:9 in 048db4d. [](commit_id = 048db4d, deletion_comment = False)

qiluo-msft

Unblock, with suggestion.

…1793) * Use MAC from EEPROM for PortChannels Signed-off-by: Andriy Moroz <[email protected]> * Use MAC from EEPROM in DEVICE_METADATA Will affect MAC for VLAN interfaces Signed-off-by: Andriy Moroz <[email protected]> * Get MAC via decode-syseeprom Signed-off-by: Andriy Moroz <[email protected]> * hw-management is now a service Signed-off-by: Andriy Moroz <[email protected]> * Add error handling for MAC fetch process Signed-off-by: Andriy Moroz <[email protected]>

jleveque · 2019-04-21T22:12:59Z

slave.mk

 	j2 -f env files/initramfs-tools/union-mount.j2 onie-image.conf > files/initramfs-tools/union-mount
 	j2 -f env files/initramfs-tools/arista-convertfs.j2 onie-image.conf > files/initramfs-tools/arista-convertfs

+	j2 files/build_templates/updategraph.service.j2 > updategraph.service


Is this line necessary? The same operation is being done in sonic_debian_extension.j2 above and there the resulting file is getting copied to its proper final destination. Why is this file getting written to the repo root? It doesn't appear to be used anywhere.

…t time (sonic-net#1749)" (sonic-net#1793) This reverts commit e56c492.

…tomatically (sonic-net#1793) #### Why I did it src/sonic-gnmi ``` * 1f064f3 - (HEAD -> 202412, origin/202412) Cherry-pick sonic-gnmi PR sonic-net#533 (sonic-net#143) (21 hours ago) [Changrong Wu] ``` #### How I did it #### How to verify it #### Description for the changelog

…tically (#26290) #### Why I did it src/sonic-sairedis ``` * 9a87a71c - (HEAD -> master, origin/master, origin/HEAD) create default hash profile for ecmp and lag (#1793) (6 hours ago) [yue-fred-gao] * 7d78093e - Support ARP passthrough (#1792) (8 hours ago) [yue-fred-gao] * 5772733c - vpp: enable bulk operations (#1794) (13 hours ago) [yue-fred-gao] * 755b853c - [nokia-c1] Add a new virtual switch type (#1779) (21 hours ago) [henry huang] ``` #### How I did it #### How to verify it #### Description for the changelog

…tically (sonic-net#26290) #### Why I did it src/sonic-sairedis ``` * 9a87a71c - (HEAD -> master, origin/master, origin/HEAD) create default hash profile for ecmp and lag (sonic-net#1793) (6 hours ago) [yue-fred-gao] * 7d78093e - Support ARP passthrough (sonic-net#1792) (8 hours ago) [yue-fred-gao] * 5772733c - vpp: enable bulk operations (sonic-net#1794) (13 hours ago) [yue-fred-gao] * 755b853c - [nokia-c1] Add a new virtual switch type (sonic-net#1779) (21 hours ago) [henry huang] ``` #### How I did it #### How to verify it #### Description for the changelog Signed-off-by: arlakshm <[email protected]>

Use MAC from EEPROM for PortChannels

e4b16e2

Signed-off-by: Andriy Moroz <[email protected]>

andriymoroz-mlnx mentioned this pull request Jun 14, 2018

Use MAC from EEPROM for PortChannels #1786

Closed

andriymoroz-mlnx force-pushed the master_mac_eeprom branch 2 times, most recently from 930e3ee to e4b16e2 Compare June 14, 2018 16:41

pavel-shirshov reviewed Jun 14, 2018

View reviewed changes

lguohan requested changes Jun 19, 2018

View reviewed changes

Use MAC from EEPROM in DEVICE_METADATA

ca3704e

Will affect MAC for VLAN interfaces Signed-off-by: Andriy Moroz <[email protected]>

qiluo-msft reviewed Jun 21, 2018

View reviewed changes

Get MAC via decode-syseeprom

8f15fd7

Signed-off-by: Andriy Moroz <[email protected]>

qiluo-msft reviewed Jun 27, 2018

View reviewed changes

qiluo-msft requested a review from taoyl-ms June 27, 2018 16:37

qiluo-msft requested changes Jun 27, 2018

View reviewed changes

hw-management is now a service

38663f4

Signed-off-by: Andriy Moroz <[email protected]>

lguohan approved these changes Jul 11, 2018

View reviewed changes

pavel-shirshov suggested changes Jul 12, 2018

View reviewed changes

Add error handling for MAC fetch process

048db4d

Signed-off-by: Andriy Moroz <[email protected]>

pavel-shirshov approved these changes Jul 18, 2018

View reviewed changes

qiluo-msft approved these changes Jul 18, 2018

View reviewed changes

lguohan merged commit dadc17d into sonic-net:master Jul 23, 2018

jleveque reviewed Apr 21, 2019

View reviewed changes

theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022

Revert "[flex-counters] Delay flex counters stats init for faster boo…

b8b2df2

…t time (sonic-net#1749)" (sonic-net#1793) This reverts commit e56c492.

Conversation

andriymoroz-mlnx commented Jun 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andriymoroz-mlnx commented Jun 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andriymoroz-mlnx Jun 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

pavel-shirshov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavel-shirshov left a comment

Choose a reason for hiding this comment

Uh oh!

qiluo-msft commented Jul 18, 2018

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

andriymoroz-mlnx commented Jun 14, 2018 •

edited

Loading

andriymoroz-mlnx Jun 20, 2018 •

edited

Loading

qiluo-msft Jun 21, 2018 •

edited

Loading

qiluo-msft Jun 27, 2018 •

edited

Loading

qiluo-msft Jun 27, 2018 •

edited

Loading